Fix external editor

[aadcg]

Description

Fixes #3215

Discussion

@hgluka I'm mostly looking for you to test the commands that this PR deals with. Set external-editor-program to a list, a string; or leave it with the default value and set VISUAL/EDITOR. Hunt for functional bugs.

Checklist:

Everything in this checklist is required for each PR. Please do not approve a PR that does not have all of these items.

• Git hygiene:
  • I have pulled from master before submitting this PR
  • There are no merge conflicts.
• I've added the new dependencies as:
  • ASDF dependencies,
  • Git submodules,

    cd /path/to/nyxt/checkout
    git submodule add https://gitlab.common-lisp.net/nyxt/py-configparser _build/py-configparser

  • and Guix dependencies.
• My code follows the style guidelines for Common Lisp code. See:
  • Norvig & Pitman's Tutorial on Good Lisp Programming Style (PDF)
  • Google Common Lisp Style Guide
• I have performed a self-review of my own code.
• My code has been reviewed by at least one peer. (The peer review to approve a PR counts. The reviewer must download and test the code.)
• Documentation:
  • All my code has docstrings and :documentations written in the aforementioned style. (It's OK to skip the docstring for really trivial parts.)
  • I have updated the existing documentation to match my changes.
  • I have commented my code in hard-to-understand areas.
  • I have updated the changelog.lisp with my changes if it's anything user-facing (new features, important bug fix, compatibility breakage).
    • Changelog update should be a separate commit.
  • I have added a migration.lisp entry for all compatibility-breaking changes.
  • (If this changes something about the features showcased on Nyxt website) I have these changes described in the new/existing article at Nyxt website or will notify one of maintainters to do so.
• Compilation and tests:
  • My changes generate no new warnings.
  • I have added tests that prove my fix is effective or that my feature works. (If possible.)
  • I ran the tests locally ((asdf:test-system :nyxt) and (asdf:test-system :nyxt/gi-gtk)) and they pass.

[hgluka]

I've tested everything (edit-with-external-editor, edit-file-with-external-editor and view-source-from-external-editor) with external-editor-program being set to string and a list.
When I used emacs or emacsclient -c -a 'emacs', everything looked good.

For some reason, I couldn't get gedit to work. When I try to run edit-file-with-external-editor, a gedit process would start and immediately become a zombie process. No errors are reported.

As for EDITOR and VISUAL, I'm having trouble getting emacs (and by extension - Nyxt) to see the environment variables I set.

As a side-note, if EDITOR and VISUAL aren't set, we should echo a message that the slot isn't set properly and gracefully handle the error that uiop:launch-program throws.

[aadcg]

For some reason, I couldn't get gedit to work. When I try to run edit-file-with-external-editor, a gedit process would start and immediately become a zombie process. No errors are reported.

How are you testing? Did you produce a binary or are you running from the REPL? What happens when you issue (uiop:launch-program '("gedit" "/full/path/to/file"))? Have you added gedit to nyxt-make-guix-cl-for-nyxt?

I'm having trouble getting emacs (and by extension - Nyxt) to see the environment variables I set.

I'm not sure if I follow. In CL, uiop:getenv is SETF-able, so (setf (uiop:getenv "VAR") "value") should work.

As a side-note, if EDITOR and VISUAL aren't set, we should echo a message

This should work as soon as I remove "gio open" from external-editor-program which, in retrospective, I don't think it's a good idea.

[hgluka]

How are you testing? Did you produce a binary or are you running from the REPL? What happens when you issue (uiop:launch-program '("gedit" "/full/path/to/file"))? Have you added gedit to nyxt-make-guix-cl-for-nyxt?

Ah, yes. I was testing from the REPL, and adding gedit to nyxt-make-guix-cl-for-nyxt in my emacs config made it work.

I'm having trouble getting emacs (and by extension - Nyxt) to see the environment variables I set.

I'm not sure if I follow. In CL, uiop:getenv is SETF-able, so (setf (uiop:getenv "VAR") "value") should work.

I didn't know that, I was looking for a setenv function and couldn't find it.

As a side-note, if EDITOR and VISUAL aren't set, we should echo a message

This should work as soon as I remove "gio open" from external-editor-program which, in retrospective, I don't think it's a good idea.

Well, that was part of the issue. Another part is that uiop:getenv sometimes returns "" instead of nil if an environment variable isn't set. This means that if you set EDITOR, but not VISUAL, you can still get an error and this message:

<INFO> [11:07:27] Issued "" to edit (#P"/home/hgluka/common-lisp/nyxt/source/dom.lisp").

The doc for uiop:getenv says this:

"Query the environment, as in C getenv.
Beware: may return empty string if a variable is present but empty;
use getenvp to return NIL in such a case."

The reason VISUAL and EDITOR are "present but empty" by default is the nyxt--pure-env function in build-scripts/nyxt-guix.el.

[aadcg]

This means that if you set EDITOR, but not VISUAL, you can still get an error

This is a good point. But, actually, why do we check for EDITOR? Nyxt can only open a GUI program, not a CLI one. Isn't that so?

[aadcg]

@hgluka the latest implementation should be clean. Can you detect a bug?

[hgluka]

@aadcg Seems good to me now.
gio open is acting weird for me. It opens the file in Notepad (inside of a WINE prefix I have for a game), but that is almost certainly a quirk of my system and/or emacs setup.