Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Honour the EDITOR and VISUAL environment variables in shell configuration #1057

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Honour the EDITOR and VISUAL environment variables in shell configuration #1057

wants to merge 4 commits into from

Commits on Jul 31, 2024

  1. rc_snippet: remove trailing whitespace 

    Trailing whitespace is often removed automatically by editors, causing
needless diff noise when reviewing.  In addition, Git tends to highlight
in red in diff output.  Remove the trailing whitespace in this file.
    @bk2204
    bk2204 committed Jul 31, 2024
    Configuration menu
    Copy the full SHA
    06a25c9 View commit details
    Browse the repository at this point in the history

  2. rc_snippet: avoid using = with [[ and ]] 

    The bash language server complains about using a single equals sign with
double bracket expressions.  Since we're not doing anything complex here
that requires a double bracket expression, let's simply use a single
bracket expression and double pipe, which is supported by all POSIX
shells.
    @bk2204
    bk2204 committed Jul 31, 2024
    Configuration menu
    Copy the full SHA
    3fa361e View commit details
    Browse the repository at this point in the history

  3. rc_snippet: use a space between parentheses for nested subshells 

    Different shells handle double parentheses differently.  bash and zsh
have them introduce an arithmetic expression if they're paired, whereas
most other shells just treat them as a nested subshell.  The bash
language server notes that a space is warranted between the parentheses
to avoid ambiguity when a nested subshell is wanted, as here, so let's
insert to make our intention clear.
    @bk2204
    bk2204 committed Jul 31, 2024
    Configuration menu
    Copy the full SHA
    0e06908 View commit details
    Browse the repository at this point in the history

  4. rc_snippet: honour EDITOR and VISUAL 

    By default, Git chooses the editor from the `GIT_EDITOR` environment
variable, then `core.editor`, then `VISUAL`, then `EDITOR`, and finally
the compiled-in default (usually `vi`, but `editor` on Debian and
Ubuntu), taking into account the terminal type if necessary.

In this snippet, we check specifically for the Git-specific options, and
if they're not set and we're within a VS Code terminal, we force the
`GIT_EDITOR` environment variable to use VS Code, overriding the user's
settings.  Presumably this is because we assume that the user wants to
use VS Code if they're running from within its terminal.

Unfortunately, this is not necessarily the case.  It's not uncommon to
use VS Code for connecting to a GitHub Codespace in order to take
advantage of port forwarding, or to allow connecting from within the
browser, without actually using or wanting to use VS Code for text
editing.  (The present author knows of multiple Vim users who do exactly
this in their professional lives.)  If the user specified the `EDITOR`
or `VISUAL` environment variables, they told us what editor they wanted
to use already: they just told that to us in a way that isn't specific
to Git.  There's no reason to make them set another configuration
specifically for Git when Git will honour their choices already.

In addition, it should be noted that `EDITOR` and `VISUAL` can contain
arbitrary POSIX shell expressions since they're passed to `sh`, so if
the user wants to override their settings inside a VS Code terminal,
they can do that by writing something like this:

  EDITOR='f() { if [ "$TERM_PROGRAM" = vscode ]; then code --wait "$@"; else vim "$@"; fi; };f'

This is completely portable and valid, and it allows the user to make
this decision for themselves.  Thus, there is no regression in
functionality if the user wants the current behaviour; they simply must
configure their dotfiles accordingly.

Respect the user's choice of editor by honouring the `EDITOR` and
`VISUAL` environment variables and not overriding `GIT_EDITOR` if they
are set.
    @bk2204
    bk2204 committed Jul 31, 2024
    Configuration menu
    Copy the full SHA
    3b9a25d View commit details
    Browse the repository at this point in the history