Avoid non-portable glob pattern in wiki#fzf#pages().#248
Merged
lervag merged 1 commit intolervag:masterfrom Oct 11, 2022
broken-pen:fix-fzf-pages
Merged
Avoid non-portable glob pattern in wiki#fzf#pages().#248lervag merged 1 commit intolervag:masterfrom broken-pen:fix-fzf-pages
wiki#fzf#pages().#248lervag merged 1 commit intolervag:masterfrom
broken-pen:fix-fzf-pages
Conversation
When `g:wiki_filetypes` contains more than one file type, the function
`wiki#fzf#pages()` only shows files that are in a subdirectory of the
wiki root (but no deeper).
The reason seems to be the glob pattern that the function generates. If
`g:wiki_filetypes == ['md']`, the pattern looks like this:
**/*.md
If `g:wiki_filetypes == ['md', 'wiki]`, the pattern looks like this:
**/*.{md,wiki}
This makes use of *brace-expansion*. The vim help page for `wildcards`
has the following to say:
> Which wildcards are supported depends on the system.
> These are the common ones: […]
It then lists some wildcards, but notably not brace-expansion.
While on my system (Ubuntu 22.04), Neovim 0.7.2 seems to support
brace-expansion, it does so in an extremely fickle manner:
```vim
glob('*.md') " Finds md files at the root
glob('**.md') " Finds md files at the root
glob('**/*.md') " Finds md files in whole tree, including root
glob('*.{md,wiki}') " Finds md and wiki files at the root
glob('**.{md,wiki}') " Finds md and wiki files at the root
glob('**/*.{md,wiki}') " Finds md and wiki files *one* directory deep
```
It appears that `**` is treated exactly like `*` except in certain
circumstances. The presence of brace-expansion seems to disable these
circumstances.
While it stands to reason that this is a bug, the child has already
fallen into the well. This commit proposes to avoid the buggy glob
pattern and instead to perform one glob per file type, and combine the
results. This may well cause some slowdown for large and nested wikis,
but I can't think of a better solution.
I've had a sweep through all occurrences of `globpath()` and it seems
none outside of `wiki#fzf#pages()` use brace-expansion.
lervag
approved these changes
Oct 11, 2022
Owner
lervag
left a comment
There was a problem hiding this comment.
Thanks, this looks very good to me! I'll make the minor adjustment mentioned in the review and merge immediately.
| return l:pages | ||
| endfunction | ||
|
|
||
| let s:slash = exists('+shellslash') && !&shellslash ? '\' : '/' |
Owner
There was a problem hiding this comment.
This seems duplicated (see line 28). I propose to remove line 28.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When
g:wiki_filetypescontains more than one file type, the functionwiki#fzf#pages()only shows files that are in a subdirectory of the wiki root (but no deeper).The reason seems to be the glob pattern that the function generates. If
g:wiki_filetypes == ['md'], the pattern looks like this:If
g:wiki_filetypes == ['md', 'wiki], the pattern looks like this:This makes use of brace-expansion. The vim help page for
wildcardshas the following to say:It then lists some wildcards, but notably not brace-expansion.
While on my system (Ubuntu 22.04), Neovim 0.7.2 seems to support brace-expansion, it does so in an extremely fickle manner:
It appears that
**is treated exactly like*except in certain circumstances. The presence of brace-expansion seems to disable these circumstances.While it stands to reason that this is a bug, the child has already fallen into the well. This commit proposes to avoid the buggy glob pattern and instead to perform one glob per file type, and combine the results. This may well cause some slowdown for large and nested wikis, but I can't think of a better solution.
I've had a sweep through all occurrences of
globpath()and it seems none outside ofwiki#fzf#pages()use brace-expansion.Please let me know if there's anything you'd like me to change or if there's another solution you'd like to see pursued. I'm not particularly attached to this one.