[PATCH 4 of 4] Contrib: vim syntax, remove unecessary ngxBlock

[Maxim Dounin]

Hello!

On Fri, Mar 03, 2017 at 11:37:36PM +0800, OOO wrote:

> I made a sample and take screenshot[1].
> This one (remove current ngxBlock) is a blocker for some other changes.
> 
> [1]:https://www.flickr.com/photos/othree/32843758310/

Ah, ok, I see.  The problem only happens if there is an 
indentation.

> And about the *ngxBlock* you talked about in the last paragraph.
> I think you are talking about curly bracket block:
> 
>     http {
>       include    conf/mime.types;
>       index    index.html index.htm index.php;
>     }
> 
> I think it's not necessary for nginx conf file syntax highlight definition.
> I can do more specific syntax highlight like ngxServerBlock for
> `server { ... }` or another for `http { ... }`.
> And made some constrain, ex: only allow *http* in *server*. Not allow
> *http* at root level.
> But it add lots of complexity. And I don't see the benefit worthy it.
> Also breaks highlight in partial conf file (which will be included by
> other conf file).

Not allowing "server" at root level will break highlighting in 
include files, agree.  But it should be still possible to check if 
there are missing / unbalanced curly brackets, and warn a user if 
tries to write "server { ..." instead of "server { ... }".  And it 
should be still possible to enforce http{} should be only at top level, 
and there should be no server{} insider another server{}.

Moreover, I think it is the only way to properly parse 
configuration for highliting.  Without matching blocks you want be 
able to properly highligh configuration directives.  For example, 
consider the following configuration:

    map $foo $bar {
        index foo;
    }

    index bar;

In this example, only "map" and "index bar" should be highlighted 
as configuration directives.  Highliting "index foo" would be an 
error, because it is not a configuration directive, but a source 
and resulting values of the map.

The same applies to matching directives.  Not seeing a directive 
ending will lead to

   server_name some-name-but-no-semicolon
   listen 80;

being misinterpreted as two configuration directives, making it 
harder for people to find the bug.  And this is actually a typical 
problem nginx users often face, as per questions on the mailing 
list.  And it would be really good for syntax highlighting to help 
them to find the bug instead of misleading them even further.

> I will fix the rewrite commit.
> But I have another question.
> Maybe you have an idea about it.
> 
>     rewrite "/foo bar/" /bazz/ last;
> 
> For now, if I fix the bug. The "/foo bar/" part will be highlight as string.
> But the /bazz/ part will be normal (no highlight).
> I think they should both be highlight as string.
> Do you have any idea?

Current behaviour is to hightlight strings everywhere 
unconditionally, and I don't think it's a problem.  Your changes 
won't introduce any regression here.

Moreover, this is something anyway will happen even if you 
specifically define matching for a particular parameter.  For 
example, consider the following configuration snippet:

    "rewrite" "/foo bar/" "/bazz/" "last";

It is identical to the snippet above from nginx point of view, but 
all the arguments and the directive name will be highlited as 
strings.  I don't really think this is something would be easy to 
resolve.

BTW, just noticed while testing the above example. ngxString now 
doesn't match at the start of a line, so the following snippet 
won't be correctly highlighted:

rewrite
"/foo/"
/bar
last;

I think the fix would be to explicitly check for whitespaces or 
newline before ngxString, like this:

diff --git a/contrib/vim/syntax/nginx.vim b/contrib/vim/syntax/nginx.vim
--- a/contrib/vim/syntax/nginx.vim
+++ b/contrib/vim/syntax/nginx.vim
@@ -13,7 +13,7 @@ syn match ngxVariable '\$\(\w\+\|{\w\+}\
 syn match ngxVariableBlock '\$\(\w\+\|{\w\+}\)' contained
 syn match ngxVariableString '\$\(\w\+\|{\w\+}\)' contained
 syn region ngxBlock start=+^+ end=+{+ skip=+\${+ contains=ngxComment,ngxDirectiveBlock,ngxVariableBlock,ngxString oneline
-syn region ngxString start=+[^:a-zA-Z>!\\@]\z(["']\)+lc=1 end=+\z1+ skip=+\\\\\|\\\z1+ contains=ngxVariableString
+syn region ngxString start=+\(^\|[ \t]\)\zs\z(["']\)+ end=+\z1+ skip=+\\\\\|\\\z1+ contains=ngxVariableString
 syn match ngxComment ' *#.*$'
 
 syn keyword ngxBoolean on

Please let me know if you have any objections to the above patch 
and/or just include it into your patch series.

-- 
Maxim Dounin
http://nginx.org/