text editor selection: follow unix defacto standards #356

Merged
6543 merged 2 commits from plgruener/tea:visual_patch into master 4 months ago

Currently, tea only supports the $EDITOR env var to open the user's preferred editor (used for reviewing pull requests).

Standard *nix practice is, however, to check for $VISUAL first and only then use $EDITOR as fallback.
This is also done by Git itself, see man git-var(1).
(Actually, the order there is $GIT_EDITOR > core.editor > $VISUAL > $EDITOR > vi)

Currently, `tea` only supports the $EDITOR env var to open the user's preferred editor (used for reviewing pull requests). Standard \*nix practice is, however, to check for $VISUAL first and only then use $EDITOR as fallback. This is also done by Git itself, see man git-var(1). (Actually, the order there is $GIT_EDITOR > core.editor > $VISUAL > $EDITOR > vi)
plgruener added 1 commit 6 months ago
a1b34187f7 support $VISUAL, prefer it over $EDITOR
lunny approved these changes 6 months ago

Maybe we also should change the fallback from vim to vi? (The latter is installed by default on many systems, unlike vim.

Maybe we also should change the fallback from `vim` to `vi`? (The latter is installed by default on many systems, unlike vim.
Poster

Maybe we also should change the fallback from vim to vi? (The latter is installed by default on many systems, unlike vim.

Yes, that would probably be the safer choice.

Should I open a new PR or edit this one?

> Maybe we also should change the fallback from `vim` to `vi`? (The latter is installed by default on many systems, unlike vim. Yes, that would probably be the safer choice. Should I open a new PR or edit this one?

@plgruener Just add it here :) Thanks!

@plgruener Just add it here :) Thanks!
noerw changed title from support $VISUAL, prefer it over $EDITOR to text editor selection: follow unix defacto standards 6 months ago
noerw added this to the v0.8.0 milestone 6 months ago
noerw added the
kind/enhancement
label 6 months ago
plgruener added 1 commit 6 months ago
noerw approved these changes 6 months ago
plgruener requested review from lunny 6 months ago
6543 approved these changes 6 months ago
Collaborator

@plgruener can you "Update" the branch?

@plgruener can you "Update" the branch?
Poster

Sorry, I didn't know I had to do anything else other than waiting (for the CI checks and then some of you merging the PR).

@plgruener can you "Update" the branch?

You mean rebasing it on top of master? I sure can, but then you'd have to review it again, wouldn't you?
Or do you mean the pending review? I don't know how I can un-request that.

Sorry, I didn't know I had to do anything else other than waiting (for the CI checks and then some of you merging the PR). > @plgruener can you "Update" the branch? You mean rebasing it on top of master? I sure can, but then you'd have to review it again, wouldn't you? Or do you mean the pending review? I don't know how I can un-request that.
Collaborator

No problem just merge current master into this feature branch, there should be a button 👇 somewhere called "update branch" witch would do the same

No problem just merge current master into this feature branch, there should be a button 👇 somewhere called "update branch" witch would do the same
plgruener force-pushed visual_patch from 38b6c826a0 to e709df6259 5 months ago
Poster

I did, but your CI is still failing.

You mean rebasing it on top of master? I sure can, but then you'd have to review it again, wouldn't you?

To clarify, this was not a technical question, but one about workflow and principle. What I meant:
as soon as I modify the PR – either by introducing a new commit, back-merging (with potential merge-conflicts) or rebasing – I instantly invalidate the (already approved) reviews, because I could have introduced another, potentially breaking or malicious, change.
So I was just wondering how you handled that.

I did, but your CI is still failing. > You mean rebasing it on top of master? I sure can, but then you'd have to review it again, wouldn't you? To clarify, this was not a technical question, but one about workflow and principle. What I meant: as soon as I modify the PR – either by introducing a new commit, back-merging (with potential merge-conflicts) or rebasing – I instantly invalidate the (already approved) reviews, because I could have introduced another, potentially breaking or malicious, change. So I was just wondering how you handled that.
Collaborator

Lgtm would still count, since the person who wil merge will have a final look anyway

Lgtm would still count, since the person who wil merge will have a final look anyway
6543 merged commit 3129e60a73 into master 4 months ago

Reviewers

noerw approved these changes 6 months ago
lunny was requested for review 6 months ago
6543 approved these changes 6 months ago
continuous-integration/drone/pr Build is passing
The pull request has been merged as 3129e60a73.
Sign in to join this conversation.
Loading…
There is no content yet.