Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(languages): add odin language #2399

Merged
merged 3 commits into from
May 5, 2022
Merged

feat(languages): add odin language #2399

merged 3 commits into from
May 5, 2022

Conversation

RobWalt
Copy link
Contributor

@RobWalt RobWalt commented May 4, 2022

This commit adds the odin language by using the grammar and highlighting
queries written by MineBills repo.

This commit resolves #2340 by just applying all the changes already
discussed in the issue.

This commit adds the odin language by using the grammar and highlighting
queries written by MineBill from github

This commit resolves #2340 by just applying all the changes already
discussed in the issue
@RobWalt
Copy link
Contributor Author

RobWalt commented May 4, 2022

  • I tested the functionality with the example also located in MineBills repo and it worked on my machine perfectly fine.

@the-mikedavis the-mikedavis self-requested a review May 4, 2022 16:42
@the-mikedavis
Copy link
Member

If you run cargo xtask docgen and commit the results it'll clear up the ❌ from the Docs CI

Copy link
Member

@the-mikedavis the-mikedavis left a comment

Choose a reason for hiding this comment

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

a few suggested changes here to bring the captures in line with the ones helix uses: https://github.com/helix-editor/helix/blob/f1a77370cf679eb0f3a58cfb12deec8ff7fc6325/book/src/themes.md#syntax-highlighting

runtime/queries/odin/highlights.scm Outdated Show resolved Hide resolved
runtime/queries/odin/highlights.scm Outdated Show resolved Hide resolved
runtime/queries/odin/highlights.scm Outdated Show resolved Hide resolved
runtime/queries/odin/highlights.scm Outdated Show resolved Hide resolved
@RobWalt
Copy link
Contributor Author

RobWalt commented May 5, 2022

Hey, I just looked into your review. Sorry for adding the queries so carelessly. I think I understand the purpose of the file a bit better now. All of your suggested changes make sense.

- mostly apply changes from review by the-mikedavis (thanks) extra
  changes listed below
- change true/false to @constant.builtin.boolean
- change comment to @comment.line
@the-mikedavis
Copy link
Member

No worries, queries typically need a bit of tuning anyways 😄

Copy link
Member

@the-mikedavis the-mikedavis left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks!

@the-mikedavis the-mikedavis merged commit 495ba40 into helix-editor:master May 5, 2022
@spindlebink
Copy link
Contributor

Highlighting still isn't working for me, although it seems like Helix is recognizing .odin files correctly and Helix lists the Odin grammar as being both fetched and compiled. Also, I'm not aware of an explicit Odin styleguide, but the Odin project itself uses tabs for indentation, so the default indentation mode should probably be set accordingly.

@the-mikedavis
Copy link
Member

Highlighting still isn't working for me

Check out hx --health odin. If it shows that you don't have highlight queries then you'll need to link your runtime directory:

helix/README.md

Lines 52 to 55 in 7b287f6

| OS | command |
|-----------|-----------|
|windows |`xcopy runtime %AppData%/helix/runtime`|
|linux/macos|`ln -s $PWD/runtime ~/.config/helix/runtime`

the Odin project itself uses tabs for indentation

Would you like to submit a PR to change it? If Odin itself uses tabs it probably makes sense to set tabs as the default (and then it can be overridden on a per-project basis with #1249).

@spindlebink
Copy link
Contributor

Linking worked, thanks! Also opened a PR regarding tabs/spaces #2464.

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.

Add Odin language support
3 participants