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

[OOO]

Hi

I send a new patch to mailing list "Contrib: vim syntax, support block region".
That one is for testing the real ngxBlock we talked in previous mail.

This patch also includes:

* http only allow in main context
* server not allow in server
* Fix the string at beginning of line

There is a way to correct highlighting the following conf:

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

By using region, just like block ( end=/;/ ).
But the syntax definition will become very huge.
Every directive:

    syn keyword ngxDirective absolute_redirect

Might becomes:

    syn keyword ngxDirective absolute_redirect nextgroup=ngxDirectiveOption

Some common directive might have other syntax group, ex: listen.
If we are going to this direction.
The patches I sent should not commit in repo now.




2017-03-04 1:24 GMT+08:00 Maxim Dounin <mdounin at mdounin.ru>:
> 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/
> _______________________________________________
> nginx-devel mailing list
> nginx-devel at nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel



-- 
OOO