Support for Yarn PnP #388
Comments
|
Example response with PnP: The response path is transformed from: To an Emacs-friendly path (on disk): |
|
@gamb I have done a similar fix just yesterday with advice-add() in tide-jump-to-filespan(). tide-get-file-buffer() seems to be a better fix point. I hesitated to publish Subj. issue here as there were still a few questions which I could not answer:
@gamb what is your experience with editing files in archives (i.e. not just a singe compressed file) from Emacs without unpacking them? Do you use tramp or tar/archive modes? May be we should have an option to "auto-unplug" yarn 2 zip package from tide-get-file-buffer() instead of opening from zip? |
|
Here is a "naive" implementation of (defun tide-get-file-buffer:override (file &optional new-file)
"Returns a buffer associated with a file. This will return the
current buffer if it matches `file'. This way we can support
temporary and indirect buffers."
(cond
((equal file (tide-buffer-file-name)) (current-buffer))
((string-match-p ".*\\.zip/.*" file)
(let* ((full-path
(replace-regexp-in-string "\\$\\$virtual.*cache/" "cache/" file))
arc-path file-path-in-arc arc-buf)
(save-match-data
(string-match "\\(.*\\.zip\\)/\\(.*\\)" full-path)
(setq arc-path (match-string 1 full-path))
(setq file-path-in-arc (match-string 2 full-path)))
(setq arc-buf (find-file-noselect arc-path))
(with-current-buffer arc-buf
(beginning-of-buffer)
(search-forward file-path-in-arc)
(archive-extract))))
((file-exists-p file) (find-file-noselect file))
(new-file (let ((buffer (create-file-buffer file)))
(with-current-buffer buffer
(set-visited-file-name file)
(basic-save-buffer)
(display-buffer buffer t))
buffer))
(t (error "Invalid file %S" file))))
(advice-add 'tide-get-file-buffer :override
#'tide-get-file-buffer:override)I tested it briefly and it seems to work well in my stings... |
|
Nice one @ramblehead :). Sorry I haven't looked at this again yet. Really quick thoughts...
|
|
Good idea on generalized support for zips! I would extend it further on all archives supported by (require 'arc-mode)
(defun tide--get-arc-path-pair (full-path)
;; not sure if "|\\\\" is need - do not have M$ Windows to check...
(let ((path-components (split-string full-path "/\\|\\\\"))
(arc-path "")
(file-path-in-arc "")
arc-found)
;; Distinguishing absolute and relative paths - i.e. trailing "/".
(unless (string-empty-p (car path-components))
(setq arc-path (car path-components)))
(setq path-components (cdr path-components))
(seq-do
(lambda (component)
(if arc-found
(setq file-path-in-arc (concat file-path-in-arc "/" component))
(setq arc-path (concat arc-path "/" component))
(when (and (file-regular-p arc-path)
(with-temp-buffer
;; 300000 is a magic number - it should
;; be more than enough to recognise any achieve
;; type header.
(insert-file-contents arc-path nil 0 300000)
(ignore-errors (archive-find-type))))
(setq arc-found t))))
path-components)
(and arc-found
(not (string-empty-p arc-path))
(not (string-empty-p file-path-in-arc))
(cons arc-path (substring file-path-in-arc 1)))))
(defun tide-get-file-buffer:override (file &optional new-file)
"Returns a buffer associated with a file. This will return the
current buffer if it matches `file'. This way we can support
temporary and indirect buffers."
(let (arc-path-pair)
(cond
((equal file (tide-buffer-file-name)) (current-buffer))
((setq arc-path-pair
(tide--get-arc-path-pair
(replace-regexp-in-string "\\$\\$virtual.*cache/" "cache/" file)))
(let ((arc-path (car arc-path-pair))
(file-path-in-arc (cdr arc-path-pair))
arc-buf)
(setq arc-buf (find-file-noselect arc-path))
(with-current-buffer arc-buf
(goto-char (point-min))
;; This should fail in nested archives.
(search-forward file-path-in-arc)
(archive-extract))))
((file-exists-p file) (find-file-noselect file))
(new-file (let ((buffer (create-file-buffer file)))
(with-current-buffer buffer
(set-visited-file-name file)
(basic-save-buffer)
(display-buffer buffer t))
buffer))
(t (error "Invalid file %S" file)))))
(advice-add 'tide-get-file-buffer :override
#'tide-get-file-buffer:override) |
|
Here is a patch for Yarn 2 paths shown in (defun tide-eldoc-maybe-show:override (text)
(with-demoted-errors "eldoc error: %s"
(and (or (tide-eldoc-display-message-p)
;; Erase the last message if we won't display a new one.
(when eldoc-last-message
(eldoc-message nil)
nil))
(eldoc-message (replace-regexp-in-string
"\\$\\$virtual.*\\(cache/\\)" "\\1" text)))))
(advice-add 'tide-eldoc-maybe-show :override
#'tide-eldoc-maybe-show:override)Both patches work well on my system. Would be great if more people could test it. Anyone interested in using tide with yarn 2 PnP packages? Edit: changed to tide-eldoc-display-message-p |
|
Shouldn't you be using the tide-specific eldoc-function you just sent a PR for? 😅 Edit: Also no need to advice a function we already own and control. Better to just update the function itself. |
|
I thought to keep two issues separate. Also, not sure if it is the best point to filter edoc messages, but can't find any better. Advice is for folks waning to test this patch quickly by copy'n'paste and evaluate in *scratch* buffer. I have been using this patch for the whole evening now :P |
|
It's merged now, so at this point it's required to use 👍 I really don't know enough about this scenario to do a solid review, but if you submit another PR for this, someone else might step up. Give it a try? |
This patch allows tide to jump to files stored in zip packages (in Yarn 2 cache) using Emacs arc-mode. See ananthakumaran#388 (This PR replaces closed PR ananthakumaran#392)
|
@asteroidcow Noticed your commit in master...asteroidcow:pnp I don't have time to try it out just now, but will later on. Do you plan to open a PR for this? |
|
@gamb the @asteroidcow code seems to be coming from my push request #394 I would suggest to use the code from the above
Separate package/advice-add is not perfect - I would rather see those changes in upstream, but as @josteink rightfully advised, further work is required to provide usage instructions, demo repositories and tests (any help on that is welcome). I am using |
|
Yup, @gamb I merged @ramblehead 's code in my local fork. It's not my work. I just needed a repo to point As an fyi, the monorepos I work with use Yarn2 PnP + workspaces, and I did have to do some more hacky and completely non-portable things to the patch to make it work with workspaces. Overall, I can jump to definitons and get type information for my code and library code. What I did notice is that I couldn't jump to a definition from one library code to another library code. Maybe the additions that @ramblehead mentioned above might mean I don't need to do this hacky stuff anymore -- will test it out this week. Here are the additional things I had to add to my (defun tide-yarn2-setup ()
"Configure executables for yarn pnp."
(interactive)
(setq
yarn-root (substring
(shell-command-to-string "yarn config get virtualFolder")
0
(- (length "/$$virtual")))
tide-node-executable "/home/johndoe/.local/bin/yarnode"
tide-tscompiler-executable (concat yarn-root "sdks/typescript/bin/tsc")
tide-tsserver-executable (concat yarn-root "sdks/typescript/bin/tsserver")
tide-tsserver-process-environment '("TSS_LOG=-level verbose -file /tmp/tss.log")
)
)
where yarn node $@ |
|
Further update on yarn 2 and yarn 3 pnp developments! I now have a new emacs package that adds yarn 2 and 3 pnp support to both tide and lsp:
(require 'yarn-pnp)
(yarn-pnp-tide-enable) ; for tide
;; or
(yarn-pnp-lsp-enable) ; for lsp
I am not sure if solving yarn-pnp -- specific problems should be done in tide or lsp code. To me it seems a bit out-of-scope for those projects. Should we instead add hooks into tide, lsp and eglot that can be used by other packages such as yarn-pnp to:
If such hooks can be added, then yarn-pnp can be refactored into a stable yarn pnp solution instead of relying on advice-add that may break at any time if advised upstream functions change. |
|
My first attempt to create an example Yarn 3 repository that uses |
Just creating this as a stub. It'd be really neat if tide could support Yarn PnP, where files are zipped inside
.yarn/cache.Quick and dirty solution that works (but relies on you already having the zipped buffer open):
(defun tide-get-file-buffer (file &optional new-file) "Returns a buffer associated with a file. This will return the current buffer if it matches `file'. This way we can support temporary and indirect buffers." (cond ((equal file (tide-buffer-file-name)) (current-buffer)) ((file-exists-p file) (find-file-noselect file)) (new-file (let ((buffer (create-file-buffer file))) (with-current-buffer buffer (set-visited-file-name file) (basic-save-buffer) (display-buffer buffer t)) buffer)) + ((string-match "\\$\\$virtual.*cache/" file) + (find-file-noselect (replace-regexp-in-string "\\$\\$virtual.*cache/" "cache/" (replace-regexp-in-string "\\.zip/" ".zip:" file)))) (t (error "Invalid file %S" file))))It is possible to
yarn unplugdependencies to unzip them, but it seems to me that we should be able to leverage Emacs ability open buffers inside zips.The text was updated successfully, but these errors were encountered: