Skip to content

Conversation

MatthiasPortzel
Copy link
Contributor

This adds the Babel ES6, JSX syntax definitions. This allows Xi to have syntax highlighting for .jsx files, popularly used by React.

Theoretically, Xi only needs the single JavaScript (Babel).sublime-syntax file. I opted to include other files from the original repository which have information relevant to the language in case a future (or current that I'm unaware of) frontend implements them. I removed other files relevant to the original repository, like .git, package.json, and tests. However, I can imagine reasons to re-evaluate this choice.

I also plan on making PRs for Swift and Typescript, the other languages I've noticed are conspicuously absent from Xi.

Update README with a link to the original repository.
Remove:
-Tests
-Screenshots
-Theme files
-packson.json
-Version update messages
With sublime_syntax_convertor, convert .tmLanguage files for JSX to .sublime-syntax.
Copy link
Member

@cmyr cmyr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, thanks.

So I'm guessing that the sublime-syntax files are derived from the .tmLanguage files. In this case, we don't need both. Would you like to try removing the .tmLanguage files and seeing if this still works as expected?

All the other files are good to include, we do use tmPreferences for doing auto-indent, for instance.

In any case, this seems like a good addition, thanks!

-Having used them to generate .sublime-syntax files, the .tmLanguage files can be removed.
@MatthiasPortzel
Copy link
Contributor Author

Just added a commit removing the .tmLanguage files. They were indeed only being used to generate the .sublime-syntax files.

I confirmed that removing the .sublime-syntax files caused the language to not be loaded. It might be possible to modify make_manifest.rs to look for .tmTheme files and link them in the manifest correctly, but that's only my theoretical understanding; I don't how one would go about doing that.

I am a little curious about the theme files. Including them caused Xi-Mac's Theme dropdown to be empty. This suggests that Xi does look at them, but also that they're not supposed to be there. I just removed the .tmTheme files, because that makes it work.

@cmyr
Copy link
Member

cmyr commented May 15, 2019

Hmm I'm slightly confused between your usage of .tmLanguage and .tmTheme.

.sublime-syntax is basically .tmLanguage reencoded as YAML, with a few extra features (.tmLanguage files were originally an XML plist).

Let's tall both sublime-syntax and .tmLanguage files syntax definitions.

These files describe a way of associating 'scope tags' with regions of a file. You can sort of think of these as being souped up css selectors; there are fuller docs here.

.tmTheme files describe a mapping from scopes to text styles. These shouldn't be part of a language definition. We include some small set of themes by default (not selected with any care, just the same defaults that syntect includes) and then we also allow the client to pass in a path where we will look for additional themes; on macOS this is ~/Library/Application Support/XiEditor/themes. (from memory, but it's something like that).

So: .tmTheme files aren't part of this process, and they don't need to be in the manifest. The whole reason we have that make_manifest.rs step is sort of hacky, but basically the syntect plugin has to state at launch-time what languages it is providing support for, and that has to be in its manifest; so whenever we add new languages we need to also rebuild that file. 🤷‍♂

Copy link
Member

@cmyr cmyr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, looking through this again: There are also the .YAML-tmXXXX files. Would you double check and see if these are included in other syntaxes in this repo, and remove them if they aren't?

@MatthiasPortzel
Copy link
Contributor Author

Okay, that makes sense. There were some .tmTheme files included in the original repository that I removed.

The YAML language files appear to also be some translation of the .tmLangauge file into yaml, although not in the same format as .sublime-syntax. When I get a chance, I’ll double check and remove them.

These files were YAML translations of .tmLanguage files, but in a different syntax than .submlime-syntax. I think they were used for generating the .tmLanguage files. Regardless, they were not being used.
@MatthiasPortzel
Copy link
Contributor Author

I removed the YAML files here, @cmyr. No rush, but wanted to make sure you saw.

@cmyr
Copy link
Member

cmyr commented May 20, 2019

Okay, going to merge this. It looks like the auto-indent rules aren't quite complete (e.g. in the following snippet the line after the conditional is not auto-indented) but the highlighting seems fine:

if (i % 15 == 0)
log "FizzBuzz";

@cmyr cmyr merged commit d82fa88 into xi-editor:master May 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants