Hello!

On Mon, Feb 20, 2017 at 12:26:52PM +0800, OOO wrote:

> The reason I send all changes in one patch is that
> I thought Vim syntax is very minor part of the entire repository.
> So I want to reduce commit.
> It's ok to send separate commits.

I certainly don't want you to send all the commits from your git
repository as separate patches, but providing separate patches for
different major changes will simplify the review. Thanks.

> * iskeyword
>
> I don't really know why the original author want to change iskeyword.
> What I can confirm is, this is for URL.
> Several months ago, I think I figure out why, but I cant remember the
> reason now.

It may make sense to ask the author directly (cc'd).

To Evan: could you please comment on this? Thanks!

> And iskeyword will change behavior of syntax highlight, but only for
> `syntax keyword`.
> I use `.` in keyword for TLSv1.1.
> So I suggest keep them if possible.
> Use `syntax iskeyword` in syntax file.
> And move iskeyword to ftplugin.

Ok, this make sense for at least "." then.

> I don't want to use the existing `&iskeyword` value like:
>
> exe "syn iskeyword ".&iskeyword.",.,/,:"
>
> Is because this value might be modified by other script or user.
> It might breaks syntax highlight.

Ok, agree. Is the distinction between win32 / non-win32 is needed
then? Just using something like "@,48-57,_,.,/,:" might be a
better option.

>
> * fastcgi
>
> Yes, I think this is not active 3rd party module.
> Currently, my rule to deal with 3rd party module is:
>
> 1. List from https://www.nginx.com/resources/wiki/modules/
> 2. One module one file ref:
> https://github.com/othree/nginx-contrib-vim/tree/master/syntax/modules
> 3. If I can find any word make sure this is deprecated, mark as
> Deprecated and use ngxDirectiveDeprecated
> 4. If the moduile don't have any dirctive, still keep the file, ex:
> https://www.nginx.com/resources/wiki/modules/zip/
>
> I think this rule need some modify.
> There are more modules seems not active now.
> But I can't confirm which is not really deprecated.
>
> Or I can just modify syntax when I know what is deprecated by user report.
>
> 2017-02-17 23:24 GMT+08:00 Maxim Dounin <mdounin@mdounin.ru>:
> > Hello!
> >
> > On Thu, Feb 16, 2017 at 11:57:35PM +0800, OOO wrote:
> >
> >> Hi
> >>
> >> This patch improves vim syntax file for nginx conf.
> >>
> >> Major changes:
> >>
> >> * Fix regexp in string might breaks location ngxBlock
> >> * Use syntax iskeyword instead of modify real Vim iskeyword (new Vim
> >> feature, can avoid change user behavior)
> >> * Update keywords, based on document[1][2]
> >> * Add/Update all 3rd party module keywords based on wiki[3][4]
> >>
> >> Full change history is on github[5].
> >>
> >> [1]:https://nginx.org/en/docs/
> >> [2]:https://github.com/othree/nginx-contrib-vim/issues/4
> >> [3]:https://www.nginx.com/resources/wiki/modules/
> >> [4]:https://github.com/othree/nginx-contrib-vim/issues/1
> >> [5]:https://github.com/othree/nginx-contrib-vim
> >>
> >> # HG changeset patch
> >> # User othree <othree@gmail.com>
> >> # Date 1487259832 -28800
> >> # Thu Feb 16 23:43:52 2017 +0800
> >> # Node ID 3f74fe213e98697c932b20b86dfdaab505a8f89b
> >> # Parent 05fd0dc8f0dc808219f727dd18a5da2f078c4073
> >> Update contrib/vim/syntax
> >>
> >> * Fix regexp in string might breaks location ngxBlock
> >> * Highlight rewrite flags
> >> * Update keywords based on latest document
> >> * Include all 3rd party modules listed on wiki
> >> * Use syntax iskeyword instead of modify iskeyword
> >
> > Thank you for the patch. Unfortunately, it fails to apply here,
> > as it was word-wrapped by your mail client. Please consider using "hg
> > email" to make sure the patch won't be corrupted in transit, see
> > http://nginx.org/en/docs/contributing_changes.html.
> >
> > Note well that it would be much better to submit separate patches
> > for different changes.
> >
> > Some additional comments below.
> >
> >> -setlocal iskeyword+=.
> >> -setlocal iskeyword+=/
> >> -setlocal iskeyword+=:
> >> +if has("patch-7.4-1142")
> >> + if has("win32")
> >> + syn iskeyword @,48-57,_,128-167,224-235,.,/,:
> >> + else
> >> + syn iskeyword @,48-57,_,192-255,.,/,:
> >> + endif
> >> +else
> >> + setlocal iskeyword+=.
> >> + setlocal iskeyword+=/
> >> + setlocal iskeyword+=:
> >> +endif
> >
> > Is it at all needed? As far as I see, syntax highliting doesn't
> > directly depends on keyword chars, so it might be better idea to just
> > remove iskeyword modifications. It will have some negative
> > effects by incorrectly matching directives in some file paths,
> > for example:
> >
> > error_log /path/to/error_log;
> >
> > will have "error_log" highlighted in the path, but this is
> > something already happens on "-" in a configuration like:
> >
> > error_log /path/to/foo-error_log;
> >
> > So probably this isn't something serious enough to worry about,
> > and just removing iskeyword modifications may be a better option.
> >
> > If you think it is really needed, it might be a better idea to use
> > something like
> >
> > if has("patch-7.4-1142")
> > exe "syn iskeyword ".&iskeyword.",.,/,:"
> > else
> > ...
> > endif
> >
> > (Seen at https://github.com/vim/vim/blob/master/runtime/syntax/sh.vim#L91).
> >
> > [...]
> >
> >> +" Asynchronous FastCGI Module https://github.com/rsms/afcgi
> >> +" Primarily a modified version of the Nginx FastCGI module which
> >> implements multiplexing of connections, allowing a single FastCGI
> >> server to handle many concurrent requests.
> >> +" syn keyword ngxDirectiveThirdParty fastcgi_bind
> >> +" syn keyword ngxDirectiveThirdParty fastcgi_buffer_size
> >> +" syn keyword ngxDirectiveThirdParty fastcgi_buffers
> >> +" syn keyword ngxDirectiveThirdParty fastcgi_busy_buffers_size
> >> +" syn keyword ngxDirectiveThirdParty fastcgi_cache
> >> +" syn keyword ngxDirectiveThirdParty fastcgi_cache_key
> >> +" syn keyword ngxDirectiveThirdParty fastcgi_cache_methods
> >> +" syn keyword ngxDirectiveThirdParty fastcgi_cache_min_uses
> >> +" syn keyword ngxDirectiveThirdParty fastcgi_cache_path
> >> +" syn keyword ngxDirectiveThirdParty fastcgi_cache_use_stale
> >> +" syn keyword ngxDirectiveThirdParty fastcgi_cache_valid
> >> +" syn keyword ngxDirectiveThirdParty fastcgi_catch_stderr
> >> +" syn keyword ngxDirectiveThirdParty fastcgi_connect_timeout
> >> +" syn keyword ngxDirectiveThirdParty fastcgi_hide_header
> >> +" syn keyword ngxDirectiveThirdParty fastcgi_ignore_client_abort
> >> +" syn keyword ngxDirectiveThirdParty fastcgi_ignore_headers
> >> +" syn keyword ngxDirectiveThirdParty fastcgi_index
> >> +" syn keyword ngxDirectiveThirdParty fastcgi_intercept_errors
> >> +" syn keyword ngxDirectiveThirdParty fastcgi_max_temp_file_size
> >> +" syn keyword ngxDirectiveThirdParty fastcgi_next_upstream
> >> +" syn keyword ngxDirectiveThirdParty fastcgi_param
> >> +" syn keyword ngxDirectiveThirdParty fastcgi_pass
> >> +" syn keyword ngxDirectiveThirdParty fastcgi_pass_header
> >> +" syn keyword ngxDirectiveThirdParty fastcgi_pass_request_body
> >> +" syn keyword ngxDirectiveThirdParty fastcgi_pass_request_headers
> >> +" syn keyword ngxDirectiveThirdParty fastcgi_read_timeout
> >> +" syn keyword ngxDirectiveThirdParty fastcgi_send_lowat
> >> +" syn keyword ngxDirectiveThirdParty fastcgi_send_timeout
> >> +" syn keyword ngxDirectiveThirdParty fastcgi_split_path_info
> >> +" syn keyword ngxDirectiveThirdParty fastcgi_store
> >> +" syn keyword ngxDirectiveThirdParty fastcgi_store_access
> >> +" syn keyword ngxDirectiveThirdParty fastcgi_temp_file_write_size
> >> +" syn keyword ngxDirectiveThirdParty fastcgi_temp_path
> >> +syn keyword ngxDirectiveThirdParty fastcgi_upstream_fail_timeout
> >> +syn keyword ngxDirectiveThirdParty fastcgi_upstream_max_fails
> >
> > This module seems to be a (likely non-working) copy of stock
> > fastcgi module with no additional features. It provides no
> > additional directives. The fastcgi_upstream_fail_timeout /
> > fastcgi_upstream_max_fails directives you've listed here are long
> > deprecated and will only print result in an error (including in
> > this module).
> >
> > [...]
> >
> > --
> > 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

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