Skip to content

Conversation

@pavoljuhas
Copy link
Contributor

Fix dirvish for auto-mounted paths where mount root does not glob.

Problem: vim glob() does not match in subdirectories of some auto mounted paths, for example,
in /home/autouser/docs/. This is probably because autouser does not glob in /home/.

Solution: use globpath instead of glob to do glob expansion only in leaf directory.

@pavoljuhas
Copy link
Contributor Author

ping @justinmk - let me know if there are any questions about this.

let paths = s:globlist(dir_esc.'*')
" Escape comma for globpath().
let dir_esc = escape(a:dir, ',')
let paths = s:globlist(dir_esc, '*')
Copy link
Owner

@justinmk justinmk Aug 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pavoljuhas the substitute(a:dir,'\[','[[]','g') was needed to handle paths with [ in the name (on Windows \ is a path separator so the [[] trick is needed to escape the [).

Do such paths work with this PR on Windows?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. I was under impression that globpath() does not expand wildcards in path values, but that is not the case.

" Escape comma for globpath().
let dir_esc = escape(a:dir, ',')
let paths = s:globlist(dir_esc, '*')
"Append dot-prefixed files. glob() cannot do both in 1 pass.
Copy link
Owner

@justinmk justinmk Aug 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we avoid 2 passes now? will this work:

let paths = s:globlist(dir_esc, '*,.[^.]*')

edit: no, the globpath() {expr} syntax is same as glob() and doesn't support "alternative" expressions.

@justinmk
Copy link
Owner

Problem: vim glob() does not match in subdirectories of some auto mounted paths, for example,
in /home/autouser/docs/. This is probably because autouser does not glob in /home/.

I don't totally understand this. Do you have a reference for this (doc, Vim issue, mailing list discussion)?


function! s:list_dir(dir) abort
" Escape for glob().
let dir_esc = escape(substitute(a:dir,'\[','[[]','g'),'{}')
Copy link
Owner

@justinmk justinmk Aug 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why don't we need to escape { and } anymore? Note also that #190 proposes to escape the \ character.

@pavoljuhas
Copy link
Contributor Author

Problem: vim glob() does not match in subdirectories of some auto mounted paths, for example,
in /home/autouser/docs/. This is probably because autouser does not glob in /home/.

I don't totally understand this. Do you have a reference for this (doc, Vim issue, mailing list discussion)?

After more testing I found the problem happens whenwildignorecase is set. On my system ls /home does not show automounted user directories, but they can be cd-ed to. I suspect wic makes glob() expand [aA][uU][tT][oO][uU][sS][eE][rR] which does not match the listing of /home. The reason globpath seemed to help is because it does not use wic (which glob does - https://github.com/vim/vim/blob/4da7a259f6b28a4f855a6fa7d0ede5e038600154/src/filepath.c#L1192-L1193).

The wic is causing problems also with current master where it merges icase-equal directories.

mkdir /tmp/a /tmp/A
touch /tmp/a/a.txt /tmp/A/b.txt
vim -i NONE -U NONE -N -c 'set wic' -c 'Dirvish /tmp/a'

b.txt
a.txt

@justinmk - are you OK with changing this PR to use the original s:globlist, but with unset wic for the glob call?

@justinmk
Copy link
Owner

justinmk commented Sep 3, 2020

The reason globpath seemed to help is because it does not use wic (which glob does - https://github.com/vim/vim/blob/4da7a259f6b28a4f855a6fa7d0ede5e038600154/src/filepath.c#L1192-L1193).

...sure enough, from :help glob() :

Unless the optional {nosuf} argument is given and is |TRUE|, the 'suffixes' and 'wildignore' options apply.
... 'wildignorecase' always applies.

classic Vim.

are you OK with changing this PR to use the original s:globlist, but with unset wic for the glob call?

Would rather use globpath() than setting/unsetting an option. Is there any disadvantage to globpath()?

@pavoljuhas
Copy link
Contributor Author

Would rather use globpath() than setting/unsetting an option. Is there any disadvantage to globpath()?

Does not seem so. globpath() needs an extra escape for , otherwise it should be similar to glob().
I will get to this over the weekend.

Fix dirvish when 'wildignorecase' is on.
Avoid combining listings of case-equivalent directories.
Escape special characters `;*?` to prevent upward search and globing.
@pavoljuhas
Copy link
Contributor Author

Here is the next iteration which keeps path escapes from master.
I have added additional escapes to prevent extra expansion for

  • , - path separator in globpath()
  • ; - upward search syntax in 'path'
  • *? - globing of the 'path' value

@justinmk justinmk merged commit da0c27e into justinmk:master Sep 6, 2020
@justinmk
Copy link
Owner

justinmk commented Sep 6, 2020

Thank you for fixing this! Nasty bug.

@pavoljuhas pavoljuhas deleted the list-with-globpath branch September 8, 2020 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants