Add Android Syntax Package (#9270)

[ismell]

This CL adds the Android Syntax package.

• I'm the package's author and/or maintainer.
• I have have read the docs.
• I have tagged a release with a semver version number.
• My package repo has a description and a README describing what it's for and how to use it.
• My package doesn't add context menu entries. *
• My package doesn't add key bindings. **
• [N/A] Any commands are available via the command palette.
• [N/A] Preferences and keybindings (if any) are listed in the menu and the command palette, and open in split view.
• If my package is a syntax it doesn't also add a color scheme. ***
• I use .gitattributes to exclude files from the package: images, test files, sublime-project/workspace.

My package is Android Syntax

There are no packages like it in Package Control.

[github-actions]

Package Review

Channel Diff

Removed (none), changed (none), added Android Syntax.

Review for Android Syntax 1.0.0

No failures

4 warnings:
- '.sublime-syntax' support has been added in build 3092 and there is no '.tmLanguage' fallback file
    File: SELinux.context.sublime-syntax
- '.sublime-syntax' support has been added in build 3092 and there is no '.tmLanguage' fallback file
    File: SELinux.te.sublime-syntax
- '.sublime-syntax' support has been added in build 3092 and there is no '.tmLanguage' fallback file
    File: init.rc.sublime-syntax
- '.sublime-syntax' support has been added in build 3092 and there is no '.tmLanguage' fallback file
    File: Android.bp.sublime-syntax


For more details on the report messages (for example how to resolve them), go to:
https://github.com/packagecontrol/st_package_reviewer/wiki

[github-actions]

Package Review

Channel Diff

Removed (none), changed (none), added Android Syntax.

Review for Android Syntax 1.0.0

No failures

4 warnings:
- '.sublime-syntax' support has been added in build 3092 and there is no '.tmLanguage' fallback file
    File: SELinux.context.sublime-syntax
- '.sublime-syntax' support has been added in build 3092 and there is no '.tmLanguage' fallback file
    File: SELinux.te.sublime-syntax
- '.sublime-syntax' support has been added in build 3092 and there is no '.tmLanguage' fallback file
    File: init.rc.sublime-syntax
- '.sublime-syntax' support has been added in build 3092 and there is no '.tmLanguage' fallback file
    File: Android.bp.sublime-syntax


For more details on the report messages (for example how to resolve them), go to:
https://github.com/packagecontrol/st_package_reviewer/wiki

[braver]

Looks good, just some questions:

• Why is the one scope source.selinux.contexts different from the others? Typically a structure like this suggests that source.selinux also exists, but it doesn't.
• Instead of putting the Apache license inside each file, just let the LICENSE file in the repo cover it.
• You might want to put your test files in a directory.
• Did you (or the agent you use) also run those tests?

[braver]

Please respond to the feedback to continue the review.

[ismell]

Thanks for the review:

Why is the one scope source.selinux.contexts different from the others? Typically a structure like this suggests that source.selinux also exists, but it doesn't.

There are SELinux context files and SELinux te files.

• scope: source.selinux.contexts
• scope: source.te
  Looks like I should rename source.te to source.selinux.te. Or should I switch to source.selinuix-te and source.selinux-contexts?

Instead of putting the Apache license inside each file, just let the LICENSE file in the repo cover it.

It's a Google open source requirement that I add it to each file.

You might want to put your test files in a directory.

Sure I can move them.

Did you (or the agent you use) also run those tests?

Yep, all the tests pass. Gemini was terrible at writing the tests, so I ended up writing most of them manually.

[braver]

Or should I switch to source.selinuix-te and source.selinux-contexts?

Yes that would be more correct. Think of them as CSS selectors if that helps. source.selinux.te means "the .te variant of the source.selinux, and source.selinux.context is related but another slightly different version of it". It sounds here like they're two different syntaxes without a shared base, but within the same space.

Thanks for all the answers. Half the reason of the review is to make sure you're actually committed to responding to feedback (from us or eventually from users).

I think we're mostly looking good here. The top level scopes of the syntaxes are not something you want to change later, so let's get that done before we merge.

Final question would be around the name of the package. Usually we advise against the "syntax" suffix, here I think it's warranted. However, shouldn't it be "syntaxes" plural?

[ismell]

I pushed a new version with the fixes: https://github.com/google/sublime-text-android-syntax/releases/tag/v1.0.1

However, shouldn't it be "syntaxes" plural?

I guess technically since it does include multiple different file formats. It just doesn't sound as nice though. I would prefer to stick with Android Syntax if that's ok with you..

Thanks again

[ismell]

Thanks!