Fix some cases in ST pattern to glob conversion#2891
Conversation
| patterns = [sublime_pattern_to_glob(pattern, True) for pattern in settings.get("folder_exclude_patterns") or []] | ||
| patterns = [ | ||
| sublime_pattern_to_glob(pattern, is_directory_pattern=True) | ||
| for pattern in settings.get("folder_exclude_patterns") or [] |
There was a problem hiding this comment.
I wonder what was the purpose of the or []?
That setting should always exist due to having default value in Preferences.sublime-settings.
There was a problem hiding this comment.
Also in general I wonder why we do this Sublime-to-glob conversion only for folder_exclude_patterns. Shouldn't it also apply to the binary_file_patterns and file_exclude_patterns?
There was a problem hiding this comment.
I've started looking at this due to this old PR that needs finishing - #2142
I will do any semi-related fixes separately (in that PR most like).
There is a lot of weird stuff going on. We match patterns using matches_pattern which uses fnmatch.fnmatch which is neither fully compatible with ST's own patterns nor with globs. So it all seems a bit broken...
There was a problem hiding this comment.
Also in general I wonder why we do this Sublime-to-glob conversion only for
folder_exclude_patterns. Shouldn't it also apply to thebinary_file_patternsandfile_exclude_patterns?
It was your change BTW: #2113 :)
There was a problem hiding this comment.
I really wonder why I put the or [] there, while for project data I used get('folder_exclude_patterns', []) instead... for project data it makes sense to have a fallback value because it must be explicitly set in project settings file.
But I guess it can't be important, because otherwise I had probably put a comment there.
Interestingly I did some considerations about binary_file_patterns and file_exclude_patterns in that linked PR before:
"binary_file_patterns" can probably be ignored, because there are no lines or columns in binary files, so language servers can't send any diagnostics for them.
Yeah, "file_exclude_patterns" could need the same treatment like in the change here, but then "file_include_patterns" (and "folder_include_patterns") should ideally also be taken into account.
2 participants
Align
sublime_pattern_to_globto match the behavior of ST implementation. In sublimehq/sublime_text#6906 I've raised some inconsistencies in ST documentation so I'm aligning with those findings. I'm assuming that the documentation will be clarified rather than implementation updated.Resolves #2112