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