search

make-files / makefiles

A library of opinionated Makefiles for popular programming languages.
https://makefiles.dev
MIT License
4 stars 5 forks source link

Prefer $VISUAL over $EDITOR when available for editing tags #116

Closed Nekroze closed 1 year ago

Nekroze commented 1 year ago

$VISUAL is the canonical store for an editor with with a graphical interface and while often set to the same value as EDITOR it may differ and most well behaving software will look for $VISUAL before looking for $EDITOR.

ezzatron commented 1 year ago

I just brushed up on what VISUAL is actually for. I had assumed it was for modern GUI vs terminal editors, but it looks like it's older than that, at least according to this StackOverflow answer:

The EDITOR editor should be able to work without use of "advanced" terminal functionality (like old ed or ex mode of vi). It was used on teletype terminals.

The main takeaway I got from it is that in order to properly support VISUAL, we need to try it first, and then fall back to EDITOR if it fails, not just prefer VISUAL.

Nekroze commented 1 year ago

to try it first, and then fall back to EDITOR if it fails

Yeah, even better! I'll get that updated.

Nekroze commented 1 year ago

tested with various versions of running make tag with VISUAL and EDITOR set or not set to valid editors or false to fail for testing and behaved as expected, VISUAL is tried first if defined and EDITOR used only if VISUAL fails, if VISUAL is not defined only editor is ever tried.

This does not affect the abort on no change behaviour which uses a diff and not the editor calls themselves.

ezzatron commented 1 year ago

Protip for future PRs - since you're a member of the org, if you create a branch on the main repo instead of a fork, anyone can try it out more easily by running rm -rf .makefiles && MF_BRANCH=<branch> make makefiles.

Feel free to hit the merge button whenever you're happy.

Nekroze commented 1 year ago

Protip for future PRs - since you're a member of the org, if you create a branch on the main repo instead of a fork, anyone can try it out more easily by running rm -rf .makefiles && MF_BRANCH=<branch> make makefiles.

Very pro, cheers didn't know about MF_BRANCH!