Expand directory wildcards (#293) #293
Conversation
juergenhoetzel
force-pushed
the
expand-directory-globs-289
branch
from
89076f9
to
9740931
Compare
* eglot.el (eglot-register-capability): prevent watching directories that contain wildcards.
juergenhoetzel
changed the title
|
Hello and thank you for the carefully crafted commit. It looks good, but since it's been a while since I've looked at this, I need you to clarify the main purpose of this commit. The commit says "expand directory wildcards: prevents watching directories that contain wildcards", and this confuses me a little. It will potentially confuse me even more in the future. Do you think this might be a slightly more adequate commit message? Your answer would help me understand if I'm getting this right. If it is, please use it instead. Regarding the code itself, I would just just use the Finally, have you assigned copyright papers for Emacs' contributions? (your patch is small enough to go in regardless). |
|
I've had a closer look at this and it seems (file-expand-wildcards "foo/**/*")We get all the files |
juergenhoetzel
force-pushed
the
expand-directory-globs-289
branch
from
9740931
to
8fac524
Compare
Previously, if the server requested a glob pattern like foo/**/* to be watched, we would just error. Now we watch foo/bar/ and foo/baz/ as if the server had requested those two watchers instead of just the one with the **. * eglot.el (eglot-register-capability): Use file-expand-wildcards to make a ** glob into multiple **-less globs.
Indeed the message was somewhat misleading. But the change does not correspond exactly to your suggestion: We only watch directories, not files. So Also the Code doesn't handle
Yes, in my original commit indeed. But I think this was an error (fixed in the amended commit). Because if we only filter the files in a expanded glob like
good suggestion: Added in the amended commit.
This is already handled in the existing code:
I signed the papers many years ago. But there seems no public lists of the persons who have signed? I'm also a tramp developer: |
It's easier to understand. Thanks.
If you like the name, I also propose that you use only this variable, i.e. assign it all the transformations in one simpler step (just one (let ((resolved-globs ;; or `dirs-to-watch', also a decent name
(delete-dups
(mapcan (eglot--lambda ((FileSystemWatcher) globPattern)
(file-name-directory
(file-expand-wildcards globPattern)))
watchers))))
...)
OK, so if we have this limitation, please add a second paragraph to the commit message explaining so, or a small FIXME-style comment to the code, or both.
Maybe, in a subsequent commit, if you think it is useful, we could import one of those functions into
Cool. The list is not public, but if you have access to a gnu.org machine (I have, but I can't find my key right now) you can supposedly consult the file. I find it easier to just ask :-) So, to summarize:
|
juergenhoetzel
force-pushed
the
expand-directory-globs-289
branch
from
8fac524
to
353694a
Compare
Previously, if the server requested a glob pattern like foo/**/* to be watched, we would just error. Now we watch foo/bar/ and foo/baz/ as if the server had requested those two watchers instead of just the one with the **. Limitation: The implementation of file-expand-wildcards doesn't handle globstars (** does match at most one path segment). * eglot.el (eglot-register-capability): Use file-expand-wildcards to make a ** glob into multiple **-less globs.
Maybe you missed my comment above:
The expanded list of files might also get very large and hit performance.
Done in the amended commit. |
Indeed you're right, I did miss it: we need to keep But anyway for, Also, there seems to be something off in the indentation inside the last progn. It seems it was already broken. Fix that if you can, please. |
Previously, if the server requested a glob pattern like foo/**/* to be watched, we would just error. Now we watch foo/bar/ and foo/baz/ as if the server had requested those two watchers instead of just the one with the **. Limitation: The implementation of file-expand-wildcards doesn't handle globstars (** does match at most one path segment). * eglot.el (eglot-register-capability): Use file-expand-wildcards to make a ** glob into multiple **-less globs.
juergenhoetzel
force-pushed
the
expand-directory-globs-289
branch
from
353694a
to
21bdbe6
Compare
Good catch!
Of course. I amended the commit. |
|
Thanks! |
Previous default (move-to-column) works on visual columns, the LSP specification and the new default (eglot-move-to-column) use "real" columns. Fixes joaotavora/eglot#293 and joaotavora/eglot#297. * eglot.el (eglot-move-to-column): New function. (eglot-move-to-column-function): Use it as default.
Previous default (move-to-column) works on visual columns, the LSP specification and the new default (eglot-move-to-column) use "real" columns. Fixes joaotavora/eglot#293 and joaotavora/eglot#297. * eglot.el (eglot-move-to-column): New function. (eglot-move-to-column-function): Use it as default.
Previously, if the server requested a glob pattern like foo/**/* to be watched, we would just error. Now we watch foo/bar/ and foo/baz/ as if the server had requested those two watchers instead of just the one with the **. As a limitation, the implementation of file-expand-wildcards doesn't fully handle ** globstars (** matches at most one path segment). * eglot.el (eglot-register-capability workspace/didChangeWatchedFiles): Use file-expand-wildcards to make a ** glob into multiple **-less globs. (#293: joaotavora/eglot#293
Previous default (move-to-column) works on visual columns, the LSP specification and the new default (eglot-move-to-column) use "real" columns. Fixes #293 and #297. * eglot.el (eglot-move-to-column): New function. (eglot-move-to-column-function): Use it as default. #293: joaotavora/eglot#293 #297: joaotavora/eglot#297
Previously, if the server requested a glob pattern like foo/**/* to be watched, we would just error. Now we watch foo/bar/ and foo/baz/ as if the server had requested those two watchers instead of just the one with the **. As a limitation, the implementation of file-expand-wildcards doesn't fully handle ** globstars (** matches at most one path segment). * eglot.el (eglot-register-capability workspace/didChangeWatchedFiles): Use file-expand-wildcards to make a ** glob into multiple **-less globs. GitHub-reference: joaotavora/eglot#293
Previous default (move-to-column) works on visual columns, the LSP specification and the new default (eglot-move-to-column) use "real" columns. Fixes joaotavora/eglot#293 and joaotavora/eglot#297. * eglot.el (eglot-move-to-column): New function. (eglot-move-to-column-function): Use it as default.
This commit prevents watching directories that contain wildcards.
I have run into the following error when using ccls, which returns a
extglob **/*globPattern:@MaskRay
**/*matches also non-source files: Could you elobarate on that?@joaotavora BTW: Thank you so much for creating this project. I'm currently porting F#-mode to LSP and eglot's simple elegant design makes it really easy for me to get started: I see it critical when today many elisp projects use tons of melpa libraries which force a foreign programming style (clojure style for example) which often also has negative effects on performance and security).