Repository: zulip/zulip-terminal
Pull request #1394 (2023-05-05 16:49:55)

Edit message in a terminal editor subprocess
Create a new command OPEN_IN_TERMINAL_EDITOR (default `ctrl o`) for editing the current message in terminal editor with python tempfile.

<!-- See README for documentation, or ask in #zulip-terminal if unclear -->
### What does this PR do, and why?

I am using helix as my terminal editor and when I write in terminal I found myself using it's shortcut. 

### External discussion & connections
<!-- [x] all that apply, specifying topic and adding numbers after # for issues/PRs -->
- [x] Discussed in **#zulip-terminal** in `Composing in terminal editor #T1394`
- [ ] Fully fixes #
- [ ] Partially fixes issue #
- [ ] Builds upon previous unmerged work in PR #
- [ ] Is a follow-up to work in PR #
- [ ] Requires merge of PR #
- [ ] Merge will enable work on #

### How did you test this?
<!-- [x] all that apply -->
- [ ] Manually - Behavioral changes
- [x] Manually - Visual changes
- [ ] Adapting existing automated tests
- [ ] Adding automated tests for new behavior (or missing tests)
- [ ] Existing automated tests should already cover this (*only a refactor of tested code*)

### Self-review checklist for each commit
- [x] It is a [minimal coherent idea](https://github.com/zulip/zulip-terminal#structuring-commits---speeding-up-reviews-merging--development)
- [x] It has a commit summary following the [documented style](https://github.com/zulip/zulip-terminal#structuring-commits---speeding-up-reviews-merging--development) (title & body)
- [ ] It has a commit summary describing the  motivation and reasoning for the change
- [x] It individually passes linting and tests
- [ ] It contains test additions for any new behavior
- [ ] It flows clearly from a previous branch commit, and/or prepares for the next commit

### Visual changes    <!-- DELETE SECTION IF NO VISUAL CHANGE -->
<!-- Zulip tips at https://zulip.readthedocs.io/en/latest/tutorials/screenshot-and-gif-software.html -->
<!-- For video, try asciinema; after uploading, embed using
[![yourtitle](https://asciinema.org/a/<id>.png)](https://asciinema.org/a/<id>)
-->

[![Composing in terminal editor](https://asciinema.org/a/3CsC3bfnwZl2gZFHYoxSP22tM.png)](https://asciinema.org/a/3CsC3bfnwZl2gZFHYoxSP22tM)

neruyzo: I update the pull request by adding an environment variable `$ZULIP_EDITOR_COMMAND` for custom command like `lapce -n -w TEMPFILE` which allow even desktop application , check and report if the program is not found.

I rename the command `OPEN_EXTERNAL_EDITOR`.
neruyzo: I updated it.

I missed before the trailing empty line that may be create by code editor, to fix that I now use `.rstrip()` to remove trailing empty line and space.

In addition to the comments, should I document $ZULIP_EDITOR_COMMAND somewhere, maybe in the README ?

I tested in most of my code editors I could use `TEMPFILE:LINE:CHARACTER` to position the cursor on opening.

Zulip terminal will try to position the cursor at the same place as before the message was edit. There is no problem when the new message is smaller than cursor position.
neruyzo: Update using shlex instead of oslex
neiljp: @mek-yt Unfortunately this great PR got a bit buried with other work over the Summer, but seems essentially good to go?

Re-reading this:
- We don't support running on windows right now, but we could also explicitly block this on windows if path handling is incomplete (or of concern)
- Alternatively, or in addition, we could sit this behind a config-file options, so that users know that one (or a specific) environment variable is in use
- Following from the previous point, if possible then a possible followup - at least for editors opening in a different location - would be to display a popup before pausing, which could indicate to the user why the screen looks the way that it does? That could show the external editor name (path?), full command used (without temp-file?) and the environment variable it was taken from.

@andersk Do you have any outstanding concerns regarding this in its current state?
mek-yt: I updated the code with setting from ZULIPRC in `editor` key, it should be ZULIPRC > `$ZULIP_EDITOR_COMMAND` > `$EDITOR` read order and print the source at launching. There are linting errors, I will check later if it is because of me.
mek-yt: @neiljp everything should be done and clean. I updated the message on opening and add

```text
external editor command 'hx' specified in zuliprc file.
```

or with new config source `environment`

```text
external editor command 'hx' specified from environment.
``` 

if there is no config in zuliprc or environment the message is

```text
external editor command '' specified from environment.
``` 

zulipbot: Heads up @mek-yt, we just merged some commits that conflict with the changes you made in this pull request! You can review this repository's [recent commits](https://github.com/zulip/zulip-terminal/commits/main) to see where the conflicts occur. Please rebase your feature branch against the `upstream/main` branch and [resolve](https://zulip.readthedocs.io/en/latest/git/troubleshooting.html#recover-from-a-git-rebase-failure) your pull request's merge conflicts accordingly.

<!-- mergeConflictWarning -->

mek-yt: I fix the conflicts.
Don't hesitate to tell me if there are some improvements need to merge this feature.
neiljp: @mek-yt Thanks for all your work on this, it's really appreciated!

I'm working on resolving some conflicts and changes necessary to integrate with other recent commits, and plan to merge soon!
neiljp: @mek-yt Current changes that I will shortly push back here before merging include:
- Split of FAQ out into a second commit, with rewording & extra notes
- Commit text adjustments
- Command description, README, and error text adjustments
- Use some more descriptive variables, and some defensive checks to avoid potential errors
- Updates of informative text to better represent the code

Generally my preference would be for a more spread-out sequence of commits, eg. we don't have environment variables as configuration, so that could be a clean separate addition.

In an ideal world I'd also request more tests, but I'd prefer we get this merged and move on with this useful feature! :tada:

Patch:
diff --git a/README.md b/README.md
index 57c1ab7..c0395a2 100644
--- a/README.md
+++ b/README.md
@@ -251,6 +251,10 @@ color-depth=256
 ## This is highly dependent on a suitable terminal emulator, and support in the selected theme
 ## Terminal emulators without this feature may show an arbitrary solid background color
 transparency=disabled
+
+## Editor: set external editor command, to edit message content
+## If not set, this falls back to the $ZULIP_EDITOR_COMMAND then $EDITOR environment variables
+# editor: nano
 ```
 
 > **NOTE:** Most of these configuration settings may be specified on the
diff --git a/docs/FAQ.md b/docs/FAQ.md
index b147444..04a5d16 100644
--- a/docs/FAQ.md
+++ b/docs/FAQ.md
@@ -13,6 +13,7 @@
   - [When are messages marked as having been read?](#when-are-messages-marked-as-having-been-read)
   - [How do I access multiple servers?](#how-do-i-access-multiple-servers)
   - [What is autocomplete? Why is it useful?](#what-is-autocomplete-why-is-it-useful)
+  - [Can I compose messages in another editor?](#can-i-compose-messages-in-another-editor)
 - Something is not working!
   - [Colors appear mismatched, don't change with theme, or look strange](#colors-appear-mismatched-dont-change-with-theme-or-look-strange)
   - [Symbols look different to in the provided screenshots, or just look incorrect](#symbols-look-different-to-in-the-provided-screenshots-or-just-look-incorrect)
@@ -373,6 +374,55 @@ through autocomplete depend upon the context automatically.
 **NOTE:** If a direct message recipient's name contains comma(s) (`,`), they
 are currently treated as comma-separated recipients.
 
+## Can I compose messages in another editor?
+
+In the `main` branch of zulip-terminal, you can now use an external editor of
+your choice to compose your messages using the `Ctrl o` hotkey.
+
+The editor command is looked for in the `ZULIP_EDITOR_COMMAND` and `EDITOR`
+environment variables (in that order), or the `editor` entry of the zuliprc
+file (which overrides both of those).
+
+It should work directly for most terminal editors with only the program name,
+eg. `vim`, `nano`, `helix`, `kakoune`, `nvim`, etc.
+
+It can also be used with a desktop editor with some constraints: the program
+must not fork or detach from the running terminal and should open in a new
+window. For this reason it may be useful to use `ZULIP_EDITOR_COMMAND` (or the
+zuliprc setting) to avoid conflict with the more widely used `EDITOR`
+environment variable. Some examples include:
+
+- [lapce](https://github.com/lapce/lapce) with `lapce -n -w`
+- [sublime-text](https://www.sublimetext.com/) with `subl -n -w`
+- [marker](https://github.com/fabiocolacio/Marker) with `marker`
+- [vim](https://github.com/vim/vim) with `vim -g -f` or `gvim -f`
+- [vscode](https://github.com/microsoft/vscode) with `code -n -w`
+
+**NOTE:** Backslashing white space (`\ `) is needed when using an executable
+containing them, for example Sublime Text on macOS can be configured with
+`/Applications/Sublime\ Text.app/Contents/SharedSupport/bin/subl`.
+
+This feature works by sending the message content back and forth using
+temporary files:
+- A temporary file is created, filled with any existing message content;
+- The editor command is run, with the filename appended (standard practice for editors);
+- You edit your text in the editor of your choice;
+- When the external editor process ends (closing the window, or quitting the terminal
+editor), the compose box will be updated with the new message content from
+the temporary file.
+
+> NOTE 1: If using a terminal-based editor, it will temporarily take over the
+> entire terminal and replace the application. However, upon exiting that
+> editor, the application should return to the terminal.
+
+> NOTE 2: Whichever type of editor you use (terminal or desktop/graphical), the
+> application will temporarily pause while you are editing. The application
+> should resume as normal once you exit the external editor and the text you
+> entered appears in the compose area.
+
+**If you have success with other editors, these examples do not work, or have
+other problems, please let us know!**
+
 ## Colors appear mismatched, don't change with theme, or look strange
 
 Some terminal emulators support specifying custom colors, or custom color
diff --git a/docs/hotkeys.md b/docs/hotkeys.md
index 9454ccc..8616227 100644
--- a/docs/hotkeys.md
+++ b/docs/hotkeys.md
@@ -97,6 +97,7 @@
 |Cycle through autocomplete suggestions in reverse|<kbd>Ctrl</kbd> + <kbd>r</kbd>|
 |Exit message compose box|<kbd>Esc</kbd>|
 |Insert new line|<kbd>Enter</kbd>|
+|Open an external editor to edit the message content|<kbd>Ctrl</kbd> + <kbd>o</kbd>|
 
 ## Editor: Navigation
 |Command|Key Combination|
diff --git a/zulipterminal/cli/run.py b/zulipterminal/cli/run.py
index 674514a..a5cce2c 100755
--- a/zulipterminal/cli/run.py
+++ b/zulipterminal/cli/run.py
@@ -33,6 +33,7 @@ from zulipterminal.version import ZT_VERSION
 class ConfigSource(Enum):
     DEFAULT = "from default config"
     ZULIPRC = "in zuliprc file"
+    ENV = "from environment"
     COMMANDLINE = "on command line"
 
 
@@ -82,6 +83,7 @@ DEFAULT_SETTINGS = {
     "maximum-footlinks": "3",
     "exit_confirmation": "enabled",
     "transparency": "disabled",
+    "editor": "",
 }
 assert DEFAULT_SETTINGS["autohide"] in VALID_BOOLEAN_SETTINGS["autohide"]
 assert DEFAULT_SETTINGS["notify"] in VALID_BOOLEAN_SETTINGS["notify"]
@@ -579,6 +581,21 @@ def main(options: Optional[List[str]] = None) -> None:
         print_setting("notify setting", zterm["notify"])
         print_setting("transparency setting", zterm["transparency"])
 
+        if zterm["editor"].source == ConfigSource.ZULIPRC:
+            editor_command = zterm["editor"].value
+            editor_config_source = ConfigSource.ZULIPRC
+        else:
+            editor_command = os.environ.get(
+                "ZULIP_EDITOR_COMMAND",
+                os.environ.get("EDITOR", ""),
+            )
+            editor_config_source = ConfigSource.ENV
+
+        print_setting(
+            "external editor command",
+            SettingData(editor_command, editor_config_source),
+        )
+
         ### Generate data not output to user, but into Controller
         # Translate valid strings for boolean values into True/False
         boolean_settings: Dict[str, bool] = dict()
@@ -605,6 +622,7 @@ def main(options: Optional[List[str]] = None) -> None:
             in_explore_mode=args.explore,
             **boolean_settings,
             debug_path=debug_path,
+            editor_command=editor_command,
         ).main()
     except ServerConnectionFailure as e:
         # Acts as separator between logs
diff --git a/zulipterminal/config/keys.py b/zulipterminal/config/keys.py
index 6f84bf2..2e9fa40 100644
--- a/zulipterminal/config/keys.py
+++ b/zulipterminal/config/keys.py
@@ -428,6 +428,11 @@ KEY_BINDINGS: Dict[str, KeyBinding] = {
         'help_text': 'Insert new line',
         'key_category': 'compose_box',
     },
+    'OPEN_EXTERNAL_EDITOR': {
+        'keys': ['ctrl o'],
+        'help_text': 'Open an external editor to edit the message content',
+        'key_category': 'compose_box',
+    },
     'FULL_RENDERED_MESSAGE': {
         'keys': ['f'],
         'help_text': 'Show/hide full rendered message (from message information)',
diff --git a/zulipterminal/core.py b/zulipterminal/core.py
index 8b53ac8..a23b159 100644
--- a/zulipterminal/core.py
+++ b/zulipterminal/core.py
@@ -69,6 +69,7 @@ class Controller:
         theme: ThemeSpec,
         color_depth: int,
         debug_path: Optional[str],
+        editor_command: str,
         in_explore_mode: bool,
         transparency: bool,
         autohide: bool,
@@ -84,6 +85,7 @@ class Controller:
         self.exit_confirmation = exit_confirmation
         self.notify_enabled = notify
         self.maximum_footlinks = maximum_footlinks
+        self.editor_command = editor_command
 
         self.debug_path = debug_path
 
diff --git a/zulipterminal/ui_tools/boxes.py b/zulipterminal/ui_tools/boxes.py
index 9856685..1a479ca 100644
--- a/zulipterminal/ui_tools/boxes.py
+++ b/zulipterminal/ui_tools/boxes.py
@@ -3,9 +3,13 @@ UI boxes for entering text: WriteBox, MessageSearchBox, PanelSearchBox
 """
 
 import re
+import shlex
+import shutil
+import subprocess
 import unicodedata
 from collections import Counter
 from datetime import datetime, timedelta
+from tempfile import NamedTemporaryFile
 from time import sleep
 from typing import Any, Callable, Dict, List, NamedTuple, Optional, Tuple
 
@@ -839,6 +843,47 @@ class WriteBox(urwid.Pile):
         elif is_command_key("MARKDOWN_HELP", key):
             self.view.controller.show_markdown_help()
             return key
+        elif is_command_key("OPEN_EXTERNAL_EDITOR", key):
+            editor_command = self.view.controller.editor_command
+
+            # None would indicate for shlex.split to read sys.stdin for Python < 3.12
+            # It should never occur in practice
+            assert isinstance(editor_command, str)
+
+            if editor_command == "":
+                self.view.controller.report_error(
+                    "No external editor command specified; "
+                    "Set 'editor' in zuliprc file, or "
+                    "$ZULIP_EDITOR_COMMAND or $EDITOR environment variables."
+                )
+                return key
+
+            editor_command_line: List[str] = shlex.split(editor_command)
+            if not editor_command_line:
+                fullpath_program = None  # A command may be specified, but empty
+            else:
+                fullpath_program = shutil.which(editor_command_line[0])
+            if fullpath_program is None:
+                self.view.controller.report_error(
+                    "External editor command not found; "
+                    "Check your zuliprc file, $EDITOR or $ZULIP_EDITOR_COMMAND."
+                )
+                return key
+            editor_command_line[0] = fullpath_program
+
+            with NamedTemporaryFile(suffix=".md") as edit_tempfile:
+                with open(edit_tempfile.name, mode="w") as edit_writer:
+                    edit_writer.write(self.msg_write_box.edit_text)
+                self.view.controller.loop.screen.stop()
+
+                editor_command_line.append(edit_tempfile.name)
+                subprocess.call(editor_command_line)
+
+                with open(edit_tempfile.name, mode="r") as edit_reader:
+                    self.msg_write_box.edit_text = edit_reader.read().rstrip()
+            self.view.controller.loop.screen.start()
+            return key
+
         elif is_command_key("SAVE_AS_DRAFT", key):
             if self.msg_edit_state is None:
                 if self.compose_box_status == "open_with_private":


Test patch:
diff --git a/tests/cli/test_run.py b/tests/cli/test_run.py
index 3586c99..fba198e 100644
--- a/tests/cli/test_run.py
+++ b/tests/cli/test_run.py
@@ -27,6 +27,8 @@ from zulipterminal.version import ZT_VERSION
 MODULE = "zulipterminal.cli.run"
 CONTROLLER = MODULE + ".Controller"
 
+os.environ["ZULIP_EDITOR_COMMAND"] = ""
+
 
 @pytest.mark.parametrize(
     "color, code",
@@ -206,6 +208,7 @@ def test_valid_zuliprc_but_no_connection(
         "   color depth setting '256' specified from default config.",
         "   notify setting 'disabled' specified from default config.",
         "   transparency setting 'disabled' specified from default config.",
+        "   external editor command '' specified from environment.",
         "\x1b[91m",
         f"Error connecting to Zulip server: {server_connection_error}.\x1b[0m",
     ]
@@ -266,6 +269,7 @@ def test_warning_regarding_incomplete_theme(
         "   color depth setting '256' specified from default config.",
         "   notify setting 'disabled' specified from default config.",
         "   transparency setting 'disabled' specified from default config.",
+        "   external editor command '' specified from environment.",
         "\x1b[91m",
         f"Error connecting to Zulip server: {server_connection_error}.\x1b[0m",
     ]
@@ -486,6 +490,7 @@ def test_successful_main_function_with_config(
         "   color depth setting '256' specified in zuliprc file.",
         "   notify setting 'enabled' specified in zuliprc file.",
         "   transparency setting 'disabled' specified from default config.",
+        "   external editor command '' specified from environment.",
     ]
     assert lines == expected_lines
 
diff --git a/tests/core/test_core.py b/tests/core/test_core.py
index 521a67d..ff6701a 100644
--- a/tests/core/test_core.py
+++ b/tests/core/test_core.py
@@ -61,6 +61,7 @@ class TestController:
             color_depth=256,
             in_explore_mode=self.in_explore_mode,
             debug_path=None,
+            editor_command="",
             **dict(
                 autohide=self.autohide,
                 notify=self.notify_enabled,