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

Add syntax highlighting and LSP configuration for Vim/Neovim #1518

Merged
merged 8 commits into from
Sep 13, 2023

Conversation

gaganchandan
Copy link
Collaborator

@gaganchandan gaganchandan commented Sep 12, 2023

This adds two files. The first is the syntax file responsible for basic highlighting. The highlight categories are based roughly on those seen in the Emacs file (swarm-mode.el). The second is the file for configuring the language server. Since it is based on Neovim's native LSP client, it only works with Neovim and not Vim.

README.md in the editor folder has also been updated to include instructions for setting these up.

@byorgey
Copy link
Member

byorgey commented Sep 12, 2023

@gaganchandan thanks so much! I can't really verify whether this works but it looks good to me overall.

There are two other related things we'll want to update:

  • A couple places in our documentation (e.g. in the README) we mention which editors are supported, so we'll want to update that---however, we are currently (update CONTRIBUTING guide, and remove info from README #1512) reshuffling some of that information so don't worry about that, we'll update it once things settle a bit.
  • We have some CI tests to ensure that our editor configurations don't get out of date when we add new keywords, commands, etc. See here for the tests:
    testEditorFiles :: TestTree

    and see here for the code to generate the configurations used in the tests:
    generateEditorKeywords :: EditorType -> IO ()

    Updating these is not a prerequisite for merging this PR. If you think you would have fun updating the tests then by all means go for it (and feel free to ask questions). But if you would be more comfortable having someone else do it, that's totally fine too. Just let us know!

@gaganchandan
Copy link
Collaborator Author

A couple places in our documentation (e.g. in the README) we mention which editors are supported, so we'll want to update that

Should I remove the changes made to the README from the PR?

About the tests, I took a look and it seems doable for me. Can I ask here if I run into any issues, or would you prefer a different medium, say, the IRC?

@byorgey
Copy link
Member

byorgey commented Sep 12, 2023

Should I remove the changes made to the README from the PR?

No, you only changed editors/README, I was talking about the top-level README.

About the tests, I took a look and it seems doable for me. Can I ask here if I run into any issues, or would you prefer a different medium, say, the IRC?

Great! Either one is fine. Asking here makes sense since the questions and answers will be archived and more easily discoverable for, say, someone adding support for additional editors in the future. But IRC is fine too, e.g. if you have quick questions that seem like they don't belong here.

@gaganchandan
Copy link
Collaborator Author

The tests are working locally on my branch, but they are failing here because editors/vim/swarm.vim does not yet exist on the main branch.

@byorgey
Copy link
Member

byorgey commented Sep 13, 2023

they are failing here because editors/vim/swarm.vim does not yet exist on the main branch.

Hmm, that shouldn't make a difference. I suspect the tests are actually failing because editors/vim/swarm.vim needs to be mentioned in swarm.cabal. Currently, that file is not included in the distribution tarball, hence it does not exist when the tarball is unpacked into the test environment. Try adding editors/vim/swarm.vim (or editors/vim/*.vim, or whatever pattern(s) you think makes sense) to the extra-source-files: field in swarm.cabal.

@gaganchandan
Copy link
Collaborator Author

You were right @byorgey. I made the changes and all the tests passed.

@byorgey
Copy link
Member

byorgey commented Sep 13, 2023

@gaganchandan Should we add swarm.lua to the extra-source-files as well? If someone does cabal install swarm they are only going to get the files mentioned in swarm.cabal. I am not sure what swarm.lua is for but if we want users to be able to easily copy or refer to it, we might want to include it.

@gaganchandan
Copy link
Collaborator Author

Yes, it does make sense to include it. I'll just add it real quick. Are there any more changes you would like?

@byorgey byorgey added the merge me Trigger the merge process of the Pull request. label Sep 13, 2023
@byorgey
Copy link
Member

byorgey commented Sep 13, 2023

No, this looks great, thanks! The CI failure looks spurious, I'm re-running that job now and hopefully this should merge once the CI passes.

@mergify mergify bot merged commit c150b05 into swarm-game:main Sep 13, 2023
9 checks passed
@byorgey
Copy link
Member

byorgey commented Sep 13, 2023

@gaganchandan , congrats on having your first PR accepted to Swarm! We appreciate the contribution and we're really glad to welcome you as part of the community. In a minute we will send you an invite to the repo (see https://github.com/swarm-game/swarm/blob/main/CONTRIBUTING.md#i-have-push-access-to-the-swarm-repository-now-what ). If you use IRC, feel free to also join the #swarm channel on Libera.Chat.

@gaganchandan
Copy link
Collaborator Author

Thank you so much!

@xsebek
Copy link
Member

xsebek commented Sep 16, 2023

@gaganchandan thanks a lot, I just set it up it and it works great! 🥳

Screenshot 2023-09-16 at 18 22 48

@xsebek xsebek mentioned this pull request Sep 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge me Trigger the merge process of the Pull request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants