chr4 / nginx.vim

Improved nginx vim plugin (incl. syntax highlighting)
450 stars 41 forks source link

Some config files not highlighting correctly #2

Closed jmreicha closed 7 years ago

jmreicha commented 7 years ago

This is cool, I've been looking for something like this for awhile. I noticed that some of my configuration files are not getting picked up by the syntax highlighter.

For example, I created a config file called test.conf and tried to apply some of the same configuration that is working in another file that is named nginx.conf.

Here is a sample snippet of the code that isn't working for me in the test.conf file.

worker_processes 1;

events {
    worker_connections 20000;
    multi_accept on;
}

http {
    # Dummy setting
    keepalive_timeout 0;

    server {
        listen 80;
    }
}
chr4 commented 7 years ago

Thanks! As nginx has no genuine file extension, the plugin tries to determine which files are nginx files using the following rules (These are the original rules of the nginx contrib plugin):

The simplest solution would be to rename your file into test.nginx.

If this is not enough, you can specify more in a custom ftdetect file, or add a custom comment # vim: ft=nginx at the top or bottom of your file and enable modeline detection with e.g. set modeline=5.

jmreicha commented 7 years ago

Gotcha, thanks for the explanation.

jmreicha commented 7 years ago

I have this in my .vimrc file for now:

autocmd BufNewFile,BufRead *.conf set syntax=nginx

We'll see how it works out. I don't really edit other files with .conf extensions outside of nginx.

justinmayer commented 7 years ago

Hey Chris. Thanks for the excellent improvement upon existing Nginx config syntax highlighting in Vim. The built-in Jinja support totally made my day. Bravo!

Apologies for commenting on a closed issue, but I wasn't sure it warranted creating a new one. I ran into this same issue with local Ansible templates that have names such as nginx-MyProject.conf and MyProject_nginx.conf, a naming convention that allows us to easily see which file we are working on in Vim (compared to a generic nginx.conf filename). It occurs to me that replacing this line:

au BufRead,BufNewFile nginx.conf set ft=nginx

... with these two lines:

au BufRead,BufNewFile nginx*.conf set ft=nginx
au BufRead,BufNewFile *nginx.conf set ft=nginx

... would allow for broader detection and worked very well in my testing. Would you consider making these minor improvements? I could submit a PR, but I thought it might be overkill for such a small change. 😄

chr4 commented 7 years ago

A pull-request is never an overkill :) - It's even most convenient for small patches, as I can just hit "merge" when they're looking good. Furthermore, you get the credit for your idea in the git commit history!

Thanks for your suggestion! I think adding the globbing is a good idea, can't think of a downside where this would match something inapropriate. Maybe a configuration file of another service also with the extension .conf, that has nginx is the name, like watch-nginx.conf.

This should be a minor issue, so I've pushed a commit with your changes. We can take them back in case someone complains :)

Thanks again!

justinmayer commented 7 years ago

Many thanks for adding this, Chris! Much appreciated! 😄