Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

No syntax highlighting if XDG_DATA_DIRS is too long #4961

Closed
luispedro opened this issue Jun 25, 2016 · 8 comments
Closed

No syntax highlighting if XDG_DATA_DIRS is too long #4961

luispedro opened this issue Jun 25, 2016 · 8 comments
Assignees
@ZyX-I
Labels
bug issues reporting wrong behavior
Milestone
0.2

Comments

@luispedro
Copy link

  • nvim --version:
    NVIM 0.1.4
    Build type: Release
    Compilation: /nix/store/6rfvkcxwglqxq08kh7w8a08p533m39gj-gcc-wrapper-5.3.0/bin/gcc -Wconversion -O2 -DNDEBUG -DDISABLE_LOG -Wall -Wextra -pedantic -Wno-unused-parameter -Wstrict-prototypes -std=gnu99 -Wvla -fstack-protector-strong -fdiagnostics-color=auto -DINCLUDE_GENERATED_DECLARATIONS -DHAVE_CONFIG_H -D_GNU_SOURCE -I/tmp/nix-build-neovim-0.1.4.drv-0/neovim-v0.1.4-src/build/config -I/tmp/nix-build-neovim-0.1.4.drv-0/neovim-v0.1.4-src/src -I/nix/store/g45z22w8gy423l28a16qqbpyjc6h8mc1-libuv-1.9.1/include -I/nix/store/r9f075g92jf3i69z7zngk4fn6kxsmdyc-libmsgpack-1.1.0/include -I/nix/store/wq0pql3gpiqj5y6j7gdigbsmkajp8gra-luajit-2.1.0-beta1/include -I/nix/store/qbhlz2b5m9z6xv2nmqz903qxiw82srcb-unibilium-1.2.0/include -I/nix/store/v6wlaxwik8j64xsx8pqw5c259kzk1g6l-libtermkey-0.18/include -I/nix/store/qibb1yiwqydp7w04mwgzb6w7qm1zadvj-neovim-libvterm-2015-11-06/include -I/nix/store/m9hvah4ccnih51ikgy9chprnq3ljsvna-jemalloc-4.1.1/include -I/nix/store/zj5k6nandjij75093g2q3f2191hvf1xi-glibc-2.23-dev/include -I/tmp/nix-build-neovim-0.1.4.drv-0/neovim-v0.1.4-src/build/src/nvim/auto -I/tmp/nix-build-neovim-0.1.4.drv-0/neovim-v0.1.4-src/build/include
    Compiled by nixbld

Optional features included (+) or not (-): +acl +iconv +jemalloc
For differences from Vim, see :help vim-differences

system vimrc file: "$VIM/sysinit.vim"
fall-back for $VIM: "
/nix/store/b8sqcgs4im2ya3hkmjd1vbyg01chivbh-neovim-0.1.4/share/nvim"

  • Vim (version: ) behaves differently?
    Yes
  • Operating system/version:
    NixOS 16.03
  • Terminal name/version:
    konsole
  • $TERM:
    xterm

Actual behaviour

No syntax highlighting if the XDG_DATA_DIRS env variable is too long (on my system it seems to be 2955).

Using :syntax on triggers an error:

Error detected while processing /nix/store/b8sqcgs4im2ya3hkmjd1vbyg01chivbh-neovim-0.1.4/share/nvim/runtime/syntax/syntax.vim:
line 42:
E216: No such group or event: filetypedetect BufRead

This was reported as a NixOS bug: NixOS/nixpkgs#15805

Expected behaviour

Syntax highlighting

Steps to reproduce using nvim -u NORC

export XDG_DATA_DIRS=<VERY LONG STRING>
nvim -u NORC
:syntax on
@justinmk
Copy link
Member

what maximum do you suggest?

@justinmk justinmk added the bug issues reporting wrong behavior label Jun 25, 2016
@justinmk justinmk added this to the todo milestone Jun 25, 2016
@luispedro
Copy link
Author

I'm not sure why you need a maximum. On the nixpkgs issue someone pointed out that vim's runtimepath internal variable handles a very long search path just fine.

@justinmk justinmk modified the milestones: 0.2, todo Jun 25, 2016
@justinmk
Copy link
Member

On the nixpkgs issue someone pointed out that vim's runtimepath internal variable handles a very long search path just fine.

That could be an accident of the fact that Vim doesn't use snprintf in many cases when working IObuff.

@ZyX-I
Copy link
Contributor

ZyX-I commented Jun 25, 2016

@luispedro Why do you think that it is length of XDG_DATA_DIRS that is responsible for the problem and not something else?

@justinmk I did not hardcode maximum size of any kind, and do not think that adding one is a good idea.

@ZyX-I
Copy link
Contributor

ZyX-I commented Jun 25, 2016

@justinmk If snprintf is not used, does not it mean overflow and not “just fine” handling? Both long &runtimepath and long $XDG_* should be just fine as long as individual directory names with suffixes (like /syntax/syntax.vim) fit into MAXPATHL.

@justinmk
Copy link
Member

@luispedro What is the longest single path in XDG_DATA_DIRS? Does removing it fix the problem?

@luispedro
Copy link
Author

Because if I manually trim XDG_DATA_DIRS down, nvim works as expected. If it's one character above 2955, it stops working.

Because of the way that NixOS works, you will end up with XDG_DATA_DIRS contains many many paths, but each individual one is a valid unix path.

@ZyX-I
Copy link
Contributor

ZyX-I commented Jun 25, 2016 •
edited

I see where is the problem: while I did not code maximum, option is then passed through option_expand and it limits the whole thing to MAXPATHL.

Actually there are two bugs:

  1. Options obtained by my code do not need option_expand at all: actually this makes paths like /foo/$BAR/baz be treated incorrectly ($BAR will be expanded, but it should be taken literally).
  2. option_expand refuses to expand if expanded option is already too long. But it checks this before it determines what value it actually will expand. So while in your case it should not expand, it still tries to do this.

ZyX-I added a commit to ZyX-I/neovim that referenced this issue Jun 25, 2016
@ZyX-I
option: Do not expand options, obtained from XDG vars
c46ec26 
It is a wrong thing to do, this makes valid variable values be treated 
incorrectly: in

    XDG_DATA_HOME='/home/$foo/.local/share'

`$foo` should be treated literally and not expanded to `foo` environment 
variable value.

Also makes option_expand not try to expand too long strings even if these too 
long strings are default values. Previously it thought that default values 
should always be expanded. Also does not try to expand NULL should it be the 
default value just in case.

Fixes neovim#4961
@ZyX-I ZyX-I mentioned this issue Jun 25, 2016
Merged
ZyX-I added a commit to ZyX-I/neovim that referenced this issue Jun 26, 2016
@ZyX-I
option: Do not expand options, obtained from XDG vars
bb04786 
It is a wrong thing to do, this makes valid variable values be treated 
incorrectly: in

    XDG_DATA_HOME='/home/$foo/.local/share'

`$foo` should be treated literally and not expanded to `foo` environment 
variable value.

Also makes option_expand not try to expand too long strings even if these too 
long strings are default values. Previously it thought that default values 
should always be expanded. Also does not try to expand NULL should it be the 
default value just in case.

Fixes neovim#4961
ZyX-I added a commit to ZyX-I/neovim that referenced this issue Jun 26, 2016
@ZyX-I
option: Do not expand options, obtained from XDG vars
eda86c7 
It is a wrong thing to do, this makes valid variable values be treated 
incorrectly: in

    XDG_DATA_HOME='/home/$foo/.local/share'

`$foo` should be treated literally and not expanded to `foo` environment 
variable value.

Also makes option_expand not try to expand too long strings even if these too 
long strings are default values. Previously it thought that default values 
should always be expanded. Also does not try to expand NULL should it be the 
default value just in case.

Fixes neovim#4961
ZyX-I added a commit to ZyX-I/neovim that referenced this issue Jul 3, 2016
@ZyX-I
option: Do not expand options, obtained from XDG vars
4d3cb63 
It is a wrong thing to do, this makes valid variable values be treated 
incorrectly: in

    XDG_DATA_HOME='/home/$foo/.local/share'

`$foo` should be treated literally and not expanded to `foo` environment 
variable value.

Also makes option_expand not try to expand too long strings even if these too 
long strings are default values. Previously it thought that default values 
should always be expanded. Also does not try to expand NULL should it be the 
default value just in case.

Fixes neovim#4961
ZyX-I added a commit to ZyX-I/neovim that referenced this issue Jul 9, 2016
@ZyX-I
option: Do not expand options, obtained from XDG vars
895f712 
It is a wrong thing to do, this makes valid variable values be treated 
incorrectly: in

    XDG_DATA_HOME='/home/$foo/.local/share'

`$foo` should be treated literally and not expanded to `foo` environment 
variable value.

Also makes option_expand not try to expand too long strings even if these too 
long strings are default values. Previously it thought that default values 
should always be expanded. Also does not try to expand NULL should it be the 
default value just in case.

Fixes neovim#4961
justinmk pushed a commit to justinmk/neovim that referenced this issue Aug 4, 2016
@ZyX-I @justinmk
option: Do not expand options, obtained from XDG vars
8427da8 
It is a wrong thing to do, this makes valid variable values be treated
incorrectly: in

    XDG_DATA_HOME='/home/$foo/.local/share'

`$foo` should be treated literally and not expanded to `foo` environment
variable value.

Also makes option_expand not try to expand too long strings even if these too
long strings are default values. Previously it thought that default values
should always be expanded. Also does not try to expand NULL should it be the
default value just in case.

Fixes neovim#4961
mgraczyk pushed a commit to mgraczyk/neovim that referenced this issue Aug 20, 2016
@ZyX-I @mgraczyk
option: Do not expand options, obtained from XDG vars
6cd1c9d 
It is a wrong thing to do, this makes valid variable values be treated 
incorrectly: in

    XDG_DATA_HOME='/home/$foo/.local/share'

`$foo` should be treated literally and not expanded to `foo` environment 
variable value.

Also makes option_expand not try to expand too long strings even if these too 
long strings are default values. Previously it thought that default values 
should always be expanded. Also does not try to expand NULL should it be the 
default value just in case.

Fixes neovim#4961
@teto teto mentioned this issue Oct 23, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ZyX-I ZyX-I

Labels
bug issues reporting wrong behavior
Projects
None yet
Milestone
0.2
Development

No branches or pull requests
3 participants
@luispedro @ZyX-I @justinmk