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@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@nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel



--
OOO
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel