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.

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

Fix history (and migration) editor path #1163

Merged
merged 6 commits into from
Nov 1, 2023
Merged

Conversation

Dhghomon
Copy link
Contributor

@Dhghomon Dhghomon commented Oct 31, 2023

We have a command \edit or \e that pulls up an editor to change the last query in the history. It won't work on Windows unless one of the two env vars is set. Unfortunately Windows doesn't have a terminal editor anymore (apparently there used to be one called edit.exe but it's gone now) so the only possible default is pulling up Notepad which is at least better than nothing and works fine.

The second change is from vim to vi on Linux, because that looks to be the default. My WSL for example has vi but not vim, so with the change to vi it will work (doesn't work at the moment). But if vim really is a preferred default then we can keep it that way.

Later noticed that the \migration edit command also pulls up the editor so unified the editor path search into a single function and now that command works in Windows too.

@Dhghomon Dhghomon changed the title Fix history editor path Fix history (and migration) editor path Oct 31, 2023
@Dhghomon Dhghomon marked this pull request as ready for review October 31, 2023 01:05
@Dhghomon Dhghomon requested review from elprans and fantix October 31, 2023 01:05
@Dhghomon Dhghomon merged commit c1a8531 into master Nov 1, 2023
@Dhghomon Dhghomon deleted the fix-history-editor-path branch November 1, 2023 16:26
elprans pushed a commit that referenced this pull request Nov 2, 2023
* Add Windows path, change vim to vi

* Add default editor names to backslash help

* +migration editor, single function for editor path

* Rephrase a bit (not just Linux)

* fmt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants