joostkremers / ebib

A BibTeX database manager for Emacs.
https://joostkremers.github.io/ebib/
BSD 3-Clause "New" or "Revised" License
275 stars 37 forks source link

Cannot view PDF on WSL Emacs. #258

Closed zyxir closed 2 years ago

zyxir commented 2 years ago

Currently I am running Emacs on WSL (Windows Subsystem for Linux). To open file in external Windows apps in WSL, you can use wslview $(wslpath -w /full/path/to/file), which works fine in my everyday usage. I can open a file with the following Emacs Lisp code:

(shell-command "wslview $(wslpath -w \"/full/path/to/file\")")

However, if I set ebib-file-associations as instructed by the manual:

(setq ebib-file-associations '(("pdf" . "wslview $(wslpath -w \"%s\")")))

The PDF files of my bib entries will not open. This problem is possibly caused by the behavior of call-process, as the code below can neither open the file as expected:

(call-process "wslview" nil nil nil "$(wslpath" "-w" "/full/path/to/file" ")")

Maybe call-process cannot handle the $() correctly. Any solutions?

zyxir commented 2 years ago

I have fixed this in this commit of my own fork. However, call-process is replaced with shell-command. I wonder if there is any advantage of call-process over shell-command?

joostkremers commented 2 years ago

I have fixed this in this commit of my own fork. However, call-process is replaced with shell-command. I wonder if there is any advantage of call-process over shell-command?

The difference is that shell-command uses a shell to execute the command, while call-process runs the command directly, so to speak. It doesn't spawn a shell first.

The advantage is that you don't need to shell-quote the arguments to call-process and that no shell is spawned, which is an additional process. The disadvantage is that you cannot use shell expansions, which is what you're running into.

I'll need to think a bit about what to do with this.

nils-schween commented 2 years ago

This might be related to an issue I filed some time ago: https://github.com/joostkremers/ebib/issues/219. I updated today, I do not see any changes, i.e. the pdf file still stays open when closing emacs and emacs does not ask to close it. I think shell-command does not introduce a regression here.

zyxir commented 2 years ago

The advantage is that you don't need to shell-quote the arguments to call-process and that no shell is spawned, which is an additional process. The disadvantage is that you cannot use shell expansions, which is what you're running into.

Maybe there is another way.

I've been using Emacs inside WSL for months, and have encountered various WSL-related issues. I have written some Emacs-Lisp utilities to deal with these issues. The purpose of the wslpath -w /full/path/to/file thing is to translate a WSL path to a Windows path, but that is not an elegant way. The alternate way that I came up about is -- to translate the WSL path inside Emacs, with Emacs-Lisp utilities. There are several steps to do this:

  1. Write a function to translate a WSL path to a Windows path. The function itself can call a wslpath process to do this, or do the conversion itself.
  2. Write an :before advice function for call-process: if the OS is WSL (I have a function to check for that in my config), and if the PROGRAM argument is something like wslview or explorer.exe, then all the path-like arguments in ARGS should be translated into Windows path, with the translator implemented in step 1.
  3. Now, ebib, as well as other packages which invoke processes, should be able to call wslview without issue.

If I am correct, all this kind of WSL-related issues could be solved, without forking any package. By the way, Ebib is a great package that have helped me a lot. Thanks for creating it!

zyxir commented 2 years ago

Maybe there is another way.

I have succeeded with my alternate way. These are my steps:

First, I already have a function zy-wsl-p that decides if Emacs is running on a WSL.

(defun zy-wsl-p ()
  "Return t if ZyEmacs is run on WSL."
  (when zy-*linux*
    (unless (boundp 'zy-wsl-p)
      (setq zy-wsl-p
        (equal
         0
         (call-process "grep" "/proc/version" t nil
                       "-q" "[Mm]icrosoft"))))
    zy-wsl-p))

Second, I impemented a set of tools to deal with paths here. zo-path-type determines the path type of a path (Unix or Windows, or cannot decide), and zo-path-to-win converts a WSL path to a Windows path.

Third, if Emacs is running on WSL, an advice function for call-process will be added:

(defun zy-wsl-win-command-p (command)
  "Return if COMMAND is a Windows command."
  (or (equal command "wslview")
      (string-match-p ".exe" command)))

(defun zy-wsl-arg-convert (program args)
  "Convert Unix paths in ARGS.

If PROGRAM is a Windows command, convert all path-like argument
in ARGS to Windows paths."
  (if (zy-wsl-win-command-p program)
      (mapcar
       (lambda (arg)
         (if (equal (zo-path-type arg 'not-a-path) 'unix)
             (zo-path-to-win arg)
           arg))
       args)
    args))

(defun zy-wsl-call-process-ad
    (call-process program &optional infile destination display
                  &rest args)
  "Call `call-process' with its arguments filtered for WSL."
  (setq args (zy-wsl-arg-convert program args))
  (apply call-process program infile destination display args))
(advice-add 'call-process :around #'zy-wsl-call-process-ad)

Similiar advice functions could be added for start-process and make-process as well, but I didn't do it, because those two functions create a process as a subprocess, and that may not work well with Windows processes outside WSL.

Finally, configure ebib like

(setq ebib-file-associations (mapcar
                              (lambda (ext)
                                `(,ext . "wslview"))
                              '("pdf" "epub" "caj" "html"))

And ebib will be able to open PDF, EPUB, CAJ and HTML files with external Windows applications.

By the way, this issue is actually not an Ebib issue, it's an issue with Emacs running on WSL. I don't know if my solution have any side-effect, but it works now at least.

joostkremers commented 2 years ago

I tend to agree that this is an issue that is larger than Ebib. If you don't mind, I would raise an issue on emacs-devel, I'd really like to know what the Emacs devs think about this.

joostkremers commented 2 years ago

This might be related to an issue I filed some time ago: #219. I updated today, I do not see any changes, i.e. the pdf file still stays open when closing emacs and emacs does not ask to close it. I think shell-command does not introduce a regression here.

Well, I haven't changed Ebib to use shell-command, @zyxir just made a local fork. From my quick look at the docs I'm unable to say whether Emacs will kill a process created with shell-command when it is killed itself.

Even if it doesn't, I'm not sure using shell-command (or async-shell-command, which would be a better choice) would be a good solution here. It does seem to be a more general Emacs problem.

zyxir commented 2 years ago

In my opinion, this may not be an issue of any program.

For Emacs, not doing shell expansion is call-process's intended behavior. If you want to do shell expansion, you should do it yourself, or use shell-command.

For Ebib, calling call-process for external program is a proper choise. I no longer consider shell-command as a good option now for its undetermined behavior.

For WSL, wslview normally accepts relative path. Not converting path is its intended behavior, because it tries to be a normal Unix program, and no other program converts its input path. If one want to convert a WSL path to a Windows path, wslpath is the suggested tool. So a shell expansion is needed to view an absolute path with wslview.

In conclusion, I think there is no one to blame for this problem. The cause of this problem is that WSL being something like a virtual machine, and Emacs is not designed to work with something outside the virtual machine. If the user do want to call Windows program inside WSL Emacs, like me did, the user should write some abstraction him/herself; or, a package to do so should be developed to assist users with this sort of demand.

zyxir commented 2 years ago

Additionally, it would be nicer if ebib-file-associations could accept a function as the file handler. For example:

(defun my-pdf-handler (file)
  "Use my customized way to deal with FILE."
  (do-something))
(setq ebib-file-associations `(("pdf" . #'my-pdf-handler)))

This way the user can use call-process, shell-command or whatever the user like to open the file. It is worth mentioning that org-ref also supports setting a function as the file opener.

I'd love to create a PR on this. I think I have to modify the docstring of ebib-file-associations and related code in ebib--call-file-viewer. The ebib manual might also be slightly modified, but that's not a part of this repository. Are there anything else?

joostkremers commented 2 years ago

In conclusion, I think there is no one to blame for this problem. The cause of this problem is that WSL being something like a virtual machine, and Emacs is not designed to work with something outside the virtual machine. If the user do want to call Windows program inside WSL Emacs, like me did, the user should write some abstraction him/herself; or, a package to do so should be developed to assist users with this sort of demand.

Yeah, that sounds like a very sensible argument.

Additionally, it would be nicer if ebib-file-associations could accept a function as the file handler.

Yeah, that actually makes sense as well. That would make it possible to solve this issue in a cleaner way.

I'd love to create a PR on this.

I'd appreciate that.

I think I have to modify the docstring of ebib-file-associations

And the :type declaration in the defcustom.

and related code in ebib--call-file-viewer. The ebib manual might also be slightly modified, but that's not a part of this repository.

Actually, it is: the source of the manual is manual/ebib.text. There's a precommit hook that converts the markdown source to texinfo/info and to html, but it requires Pandoc.

I can update the manual, if you prefer.

Are there anything else?

Not that I can think of.

zyxir commented 2 years ago

Do you have any time to review my pull request? thanks!