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

Generate vim syntax based on the documentation #99

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

quinox
Copy link
Contributor

@quinox quinox commented May 4, 2024

Script for generating a fully up-to-date-and-correct vim syntax file based on the documentation.

Some notes:

  • I took special care to make vim not show colors when the configuration is wrong. Many syntax files I looked at for inspiration don't do this, fe. the nginx syntax highlights the listen directive in all possible scopes even though nginx doesn't allow this. A lot of the examples I looked at also used syn Keyword, but that means the words get highlighted no matter where it is on the line whereas with FlashMQ we know the option has to be the first word on the line.
  • I'm not using the Error type: I think it's nicer to err on the side of non-aggression. For example, an option could be accidentally left out of the manual, or the syntax file could be slightly older than FlashMQ itself. In these cases you don't want to render sections of the config file in red.
  • There are a lot of different ways one can set up a syntax highligher. This is just the one I settled on, I'm not claiming it's in any particular way the right way of doing it. It seems to be deviating from most of the syntax files on my system.
  • Seeing how the vim syntax highlight system works it's clear you shouldn't strife for perfection but accept "good enough". The nginx example I mentioned isn't all bad: nginx itself will report a clear error if you do it wrong anyway, and it greatly simplifies the vim file.
  • For the documentation you can run :help mysyntaxfile inside vim (or look at the online documentation)
  • I gave some of your config options new IDs to play nice with this script
  • I have no idea how an apt package is supposed to roll this out on a system, you'll have to do some research for that yourself.
  • I'm using \s* in the script but I haven't checked to see how the FlashMQ parser behaves wrt. tabs and spaces. If you generally don't accept tabs you can replace them with *

How to use the generated file

Modify these instructions according to the flavor of vim you're using:

$ cat ~/.config/nvim/ftdetect/flashmq.vim
au BufNewFile,BufRead /etc/flashmq/*.conf,flashmq.conf         setf flashmq

$ ls -al ~/.config/nvim/syntax/flashmq.vim
/home/quinox/.config/nvim/syntax/flashmq.vim -> /home/quinox/projects/FlashMQ/man/flashmq.vim

Examples

flashmq_vim_good

flashmq_vim_bad

@bigsmoke
Copy link
Contributor

bigsmoke commented May 4, 2024

I'm stoked to try this! 🎁

@halfgaar
Copy link
Owner

halfgaar commented May 5, 2024

Great addition 👍

I would like the .deb to install this globally. Step one is easy:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index a1e7d49..0cad90f 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -44,6 +44,7 @@ install(FILES flashmq.conf DESTINATION "/etc/flashmq")
 install(FILES debian/flashmq.service DESTINATION "/lib/systemd/system")
 install(FILES man/flashmq.conf.5 DESTINATION "/usr/share/man/man5")
 install(FILES man/flashmq.1 DESTINATION "/usr/share/man/man1")
+install(FILES man/flashmq.vim DESTINATION "/usr/share/vim/vimfiles/syntax")

And that installs it, even if the dir doesn't exist. From what I can tell, the vimfiles dir is for external installs. The vim82 (on my test system) is for what vim installs itself. Like nginx.vim, that is not provided by Nginx, but by vim.

But, I'm having issues getting the file type detection to work. I can edit /usr/share/vim/vim82/filetype.vim, but that makes no sense from a packaging perspective.

I'm also having a hard time to make the modeline work (# vim: set ft=flashmq). I would add it to the default config.

Also small note: I have to add a space after a keyword to make it green. I don't have to do that for Nginx config files.

@quinox
Copy link
Contributor Author

quinox commented May 5, 2024

I'm also having a hard time to make the modeline work (# vim: set ft=flashmq). I would add it to the default config.

This works for me, it switches syntax when I open the file:

# vim: set ft=flashmq:

bridge {
        listen 123
}

I can't offer more help, I never use that feature of vim. Double-check set modeline? and set modelines, or find out what limitations your vim flavor has when it comes to modeline.

Also small note: I have to add a space after a keyword to make it green. I don't have to do that for Nginx config files.

I see what you mean, yeah it's nice if that works. I pushed a fix for it. The problem is that I'm not using Keyword since it's too liberal for my taste. The solution was to add a second regular expression that matches a line ending with a directive.

@quinox quinox force-pushed the feat-vim-syntax branch 2 times, most recently from c40980b to a32512e Compare May 5, 2024 14:51
@bigsmoke
Copy link
Contributor

bigsmoke commented May 6, 2024

@halfgaar, Does your vim have the modeline option enabled? The modeline option is often disabled for root (or even globally) for security reasons. See, for example: https://superuser.com/a/1427691

More info in :help modelines

@halfgaar
Copy link
Owner

Note, @bigsmoke added a feature recently, now merged in master, which adds a listen and bridge option tcp_nodelay (two options). Because you prefixed stuff with listen__, that may need updating.

I haven't yet continued with my attempt to get the file type selection in order. I'd like to fix that for the .deb builds before I merge it in.

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.

3 participants