Skip to content

Preserve and clear the saved current line properly #1259

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

Merged
merged 7 commits into from
Jan 2, 2020
Merged

Preserve and clear the saved current line properly #1259

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5e9ddaf
Clear saved current line in RevertLine
daxian-dbw Dec 20, 2019
1798e15
Add tests
daxian-dbw Dec 20, 2019
cd420fb
Clear the saved current line in a more general way
daxian-dbw Dec 31, 2019
6dcce56
Add more tests
daxian-dbw Dec 31, 2019
efecd89
Preserve the _savedCurrentLine and _currentHistoryIndex while in a se…
daxian-dbw Jan 1, 2020
33ea82f
Clone the saved edits when restoring the saved current line
daxian-dbw Jan 1, 2020
30886b2
Rename _allHistoryCommandCount to _anyHistoryCommandCount
daxian-dbw Jan 1, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 19 additions & 9 deletions PSReadLine/History.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ public class HistoryItem
private int _getNextHistoryIndex;
private int _searchHistoryCommandCount;
private int _recallHistoryCommandCount;
private int _anyHistoryCommandCount;
private string _searchHistoryPrefix;
// When cycling through history, the current line (not yet added to history)
// is saved here so it can be restored.
Expand All @@ -113,6 +114,13 @@ public class HistoryItem
"password|asplaintext|token|key|secret",
RegexOptions.Compiled | RegexOptions.IgnoreCase);

private void ClearSavedCurrentLine()
{
_savedCurrentLine.CommandLine = null;
_savedCurrentLine._edits = null;
_savedCurrentLine._undoEditIndex = 0;
}

private AddToHistoryOption GetAddToHistoryOption(string line)
{
// Whitespace only is useless, never add.
Expand Down Expand Up @@ -217,9 +225,7 @@ private string MaybeAddToHistory(
// to recall the saved line.
if (_getNextHistoryIndex == 0)
{
_savedCurrentLine.CommandLine = null;
_savedCurrentLine._edits = null;
_savedCurrentLine._undoEditIndex = 0;
ClearSavedCurrentLine();
}
return result;
}
Expand Down Expand Up @@ -469,7 +475,7 @@ private void UpdateFromHistory(HistoryMoveCursor moveCursor)
if (_currentHistoryIndex == _history.Count)
{
line = _savedCurrentLine.CommandLine;
_edits = _savedCurrentLine._edits;
_edits = new List<EditItem>(_savedCurrentLine._edits);
_undoEditIndex = _savedCurrentLine._undoEditIndex;
}
else
Expand Down Expand Up @@ -505,6 +511,7 @@ private void SaveCurrentLine()
// to check if we need to load history from another sessions now.
MaybeReadHistoryFile();

_anyHistoryCommandCount += 1;
if (_savedCurrentLine.CommandLine == null)
{
_savedCurrentLine.CommandLine = _buffer.ToString();
Expand Down Expand Up @@ -633,7 +640,7 @@ private void HistorySearch(int direction)
continue;
}

var line = newHistoryIndex == _history.Count ? _savedCurrentLine.CommandLine : _history[newHistoryIndex].CommandLine;
var line = _history[newHistoryIndex].CommandLine;
Copy link
Member Author

@daxian-dbw daxian-dbw Dec 20, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The newHistoryIndex == _history.Count is removed because that was already checked above:

if (newHistoryIndex < 0 || newHistoryIndex >= _history.Count)
{
    break;
}

if (line.StartsWith(_searchHistoryPrefix, Options.HistoryStringComparison))
{
if (Options.HistoryNoDuplicates)
Expand Down Expand Up @@ -681,6 +688,12 @@ public static void BeginningOfHistory(ConsoleKeyInfo? key = null, object arg = n
/// Move to the last item (the current input) in the history.
/// </summary>
public static void EndOfHistory(ConsoleKeyInfo? key = null, object arg = null)
{
_singleton.SaveCurrentLine();
GoToEndOfHistory();
}

private static void GoToEndOfHistory()
{
_singleton._currentHistoryIndex = _singleton._history.Count;
_singleton.UpdateFromHistory(HistoryMoveCursor.ToEnd);
Expand Down Expand Up @@ -839,7 +852,7 @@ private void InteractiveHistorySearchLoop(int direction)
else if (function == Abort)
{
// Abort search
EndOfHistory();
GoToEndOfHistory();
break;
}
else
Expand Down Expand Up @@ -882,9 +895,6 @@ private void InteractiveHistorySearch(int direction)
Render(); // Render prompt
InteractiveHistorySearchLoop(direction);

_hashedHistory = null;
_currentHistoryIndex = _history.Count;

_emphasisStart = -1;
_emphasisLength = 0;

Expand Down
18 changes: 10 additions & 8 deletions PSReadLine/ReadLine.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -474,6 +474,7 @@ private string InputLoop()
var tabCommandCount = _tabCommandCount;
var searchHistoryCommandCount = _searchHistoryCommandCount;
var recallHistoryCommandCount = _recallHistoryCommandCount;
var anyHistoryCommandCount = _anyHistoryCommandCount;
var yankLastArgCommandCount = _yankLastArgCommandCount;
var visualSelectionCommandCount = _visualSelectionCommandCount;
var moveToLineCommandCount = _moveToLineCommandCount;
Expand Down Expand Up @@ -515,23 +516,23 @@ private string InputLoop()
_emphasisStart = -1;
_emphasisLength = 0;
Render();
_currentHistoryIndex = _history.Count;
}
_searchHistoryCommandCount = 0;
_searchHistoryPrefix = null;
}
if (recallHistoryCommandCount == _recallHistoryCommandCount)
{
if (_recallHistoryCommandCount > 0)
{
_currentHistoryIndex = _history.Count;
}
_recallHistoryCommandCount = 0;
}
if (searchHistoryCommandCount == _searchHistoryCommandCount &&
recallHistoryCommandCount == _recallHistoryCommandCount)
if (anyHistoryCommandCount == _anyHistoryCommandCount)
{
_hashedHistory = null;
if (_anyHistoryCommandCount > 0)
{
ClearSavedCurrentLine();
_hashedHistory = null;
_currentHistoryIndex = _history.Count;
}
_anyHistoryCommandCount = 0;
}
if (visualSelectionCommandCount == _visualSelectionCommandCount && _visualSelectionCommandCount > 0)
{
Expand Down Expand Up @@ -717,6 +718,7 @@ private void Initialize(Runspace runspace, EngineIntrinsics engineIntrinsics)
_yankLastArgCommandCount = 0;
_tabCommandCount = 0;
_recallHistoryCommandCount = 0;
_anyHistoryCommandCount = 0;
_visualSelectionCommandCount = 0;
_hashedHistory = null;

Expand Down
3 changes: 3 additions & 0 deletions PSReadLine/ReadLine.vi.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -835,6 +835,7 @@ private static void DeleteRange(int first, int last, Action<ConsoleKeyInfo?, obj
/// </summary>
public static void ViSearchHistoryBackward(ConsoleKeyInfo? key = null, object arg = null)
{
_singleton.SaveCurrentLine();
_singleton.StartSearch(backward: true);
}

Expand All @@ -843,6 +844,7 @@ public static void ViSearchHistoryBackward(ConsoleKeyInfo? key = null, object ar
/// </summary>
public static void SearchForward(ConsoleKeyInfo? key = null, object arg = null)
{
_singleton.SaveCurrentLine();
_singleton.StartSearch(backward: false);
}

Expand All @@ -857,6 +859,7 @@ public static void RepeatSearch(ConsoleKeyInfo? key = null, object arg = null)
return;
}

_singleton._anyHistoryCommandCount++;
_singleton.HistorySearch();
}

Expand Down
32 changes: 32 additions & 0 deletions test/HistoryTest.VI.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,38 @@ public void ViHistoryRepeat()
));
}

[SkippableFact]
public void ViHistoryCommandMix()
{
TestSetup(KeyMode.Vi);

// Clear history in case the above added some history (but it shouldn't)
SetHistory();
Test( " ", Keys( ' ', _.UpArrow, _.DownArrow ) );

// Mix history search, repeat, and recall.
// Mix different history commands to verify that the saved current line and
// the history index stay the same while in a series of history commands.

SetHistory("bar1", "bar2", "bar3", "bar4", "bar5");
Test("first", Keys(
"first", _.Escape, _.Slash, "bar", _.Enter,
CheckThat(() => AssertLineIs("bar5")),
_.DownArrow, CheckThat(() => AssertLineIs("first")),
_.Slash, "bar", _.Enter,
CheckThat(() => AssertLineIs("bar5")),
"nn", CheckThat(() => AssertLineIs("bar3")),
"N", CheckThat(() => AssertLineIs("bar4")),
"N", CheckThat(() => AssertLineIs("bar5")),
"nnn", CheckThat(() => AssertLineIs("bar2")),
_.UpArrow, CheckThat(() => AssertLineIs("bar1")),
_.DownArrow, CheckThat(() => AssertLineIs("bar2")),
_.DownArrow, CheckThat(() => AssertLineIs("bar3")),
_.DownArrow, CheckThat(() => AssertLineIs("bar4")),
_.DownArrow, CheckThat(() => AssertLineIs("bar5")),
_.DownArrow));
}

[SkippableFact]
public void ViMovementAfterHistory()
{
Expand Down
149 changes: 147 additions & 2 deletions test/HistoryTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -409,8 +409,40 @@ public void HistoryRecallCurrentLine()
{
TestSetup(KeyMode.Cmd);

// Recall history backward and forward.
SetHistory("echo foo", "echo bar");
Test("ec", Keys("ec", _.UpArrow, _.UpArrow, _.DownArrow, _.DownArrow));
Test("ec", Keys(
"ec",
_.UpArrow, CheckThat(() => AssertLineIs("echo bar")),
_.UpArrow, CheckThat(() => AssertLineIs("echo foo")),
_.DownArrow, CheckThat(() => AssertLineIs("echo bar")),
_.DownArrow));

// Verify that the saved current line gets reset when the line gets edited.
// Recall history, then edit the line, and recall again.
SetHistory("echo foo", "echo bar");
Test("get", Keys(
"ec", _.UpArrow,
_.DownArrow, CheckThat(() => AssertLineIs("ec")),
_.Escape, "get", _.UpArrow, _.DownArrow));

// Recall history, then edit the line, and recall again.
SetHistory("echo foo", "echo bar");
Test("ge", Keys(
"ec", _.UpArrow,
_.DownArrow, CheckThat(() => AssertLineIs("ec")),
_.Backspace, _.Backspace, "ge", CheckThat(() => AssertLineIs("ge")),
_.UpArrow, _.DownArrow));

// Recall history, then edit the line, and recall again.
SetHistory("echo foo", "echo bar");
Test("", Keys(
"ec", _.UpArrow,
_.DownArrow, CheckThat(() => AssertLineIs("ec")),
"h", CheckThat(() => AssertLineIs("ech")),
_.UpArrow, CheckThat(() => AssertLineIs("echo bar")),
_.DownArrow, CheckThat(() => AssertLineIs("ech")),
_.Escape));
}

[SkippableFact]
Expand All @@ -420,8 +452,121 @@ public void HistorySearchCurrentLine()
new KeyHandler("UpArrow", PSConsoleReadLine.HistorySearchBackward),
new KeyHandler("DownArrow", PSConsoleReadLine.HistorySearchForward));

// Search history backward and forward.
SetHistory("echo foo", "echo bar");
Test("ec", Keys(
"ec",
_.UpArrow, CheckThat(() => AssertLineIs("echo bar")),
_.UpArrow, CheckThat(() => AssertLineIs("echo foo")),
_.DownArrow, CheckThat(() => AssertLineIs("echo bar")),
_.DownArrow));

// Verify that the saved current line gets reset when the line gets edited.
// Search history, then edit the line, and search again.
SetHistory("echo foo", "echo bar");
Test("echo ", Keys(
"ec", _.UpArrow,
_.DownArrow, CheckThat(() => AssertLineIs("ec")),
_.Escape, "echo ",
_.UpArrow, CheckThat(() => AssertLineIs("echo bar")),
_.DownArrow));

// Search history, then edit the line, and search again.
SetHistory("echo foo", "echo bar");
Test("echo", Keys(
"ec", _.UpArrow, _.DownArrow,
"ho", CheckThat(() => AssertLineIs("echo")),
_.UpArrow, _.DownArrow));

// Search history, then edit the line, and search again.
SetHistory("echo foo", "echo bar");
Test("e", Keys(
"ec", _.UpArrow, _.DownArrow,
_.Backspace, CheckThat(() => AssertLineIs("e")),
_.UpArrow, _.DownArrow));

// Search history, then edit the line, and search again.
SetHistory("echo foo", "echo bar");
Test("", Keys(
"ec", _.UpArrow, _.DownArrow, "ho f",
_.UpArrow, CheckThat(() => AssertLineIs("echo foo")),
_.DownArrow, CheckThat(() => AssertLineIs("echo f")),
_.Escape));
}

[SkippableFact]
public void HistorySavedCurrentLine()
{
TestSetup(KeyMode.Cmd,
new KeyHandler("F3", PSConsoleReadLine.BeginningOfHistory),
new KeyHandler("Shift+F3", PSConsoleReadLine.EndOfHistory));

// Mix different history commands to verify that the saved current line and
// the history index stay the same while in a series of history commands.

SetHistory("echo foo", "echo bar");
Test("ec", Keys("ec", _.UpArrow, _.UpArrow, _.DownArrow, _.DownArrow));
Test("ec", Keys(
"ec",
_.UpArrow, CheckThat(() => AssertLineIs("echo bar")),
_.F3, CheckThat(() => AssertLineIs("echo foo")),
_.DownArrow, CheckThat(() => AssertLineIs("echo bar")),
_.DownArrow));

SetHistory("echo foo", "get zoo", "echo bar");
Test("ec", Keys(
"ec",
_.UpArrow, CheckThat(() => AssertLineIs("echo bar")),
_.F3, CheckThat(() => AssertLineIs("echo foo")),
_.Shift_F3));

SetHistory("echo foo", "get zoo", "echo bar");
Test("e", Keys(
"e",
_.UpArrow, CheckThat(() => AssertLineIs("echo bar")),
_.UpArrow, CheckThat(() => AssertLineIs("get zoo")),
_.Shift_F3));

SetHistory("echo foo", "get zoo", "echo bar");
Test("ech", Keys(
"ech",
_.F8, CheckThat(() => AssertLineIs("echo bar")),
_.F3, CheckThat(() => AssertLineIs("echo foo")),
_.DownArrow, CheckThat(() => AssertLineIs("get zoo")),
_.DownArrow, CheckThat(() => AssertLineIs("echo bar")),
_.DownArrow));

SetHistory("echo foo", "get zoo", "echo bar");
Test("ech", Keys(
"ech",
_.F8, CheckThat(() => AssertLineIs("echo bar")),
_.F8, CheckThat(() => AssertLineIs("echo foo")),
_.Shift_F3));

SetHistory("echo foo", "get bar", "echo f");
Test("ec", Keys(
"ec",
_.UpArrow, CheckThat(() => AssertLineIs("echo f")),
_.F8, CheckThat(() => AssertLineIs("echo foo")),
_.Shift_F8, CheckThat(() => AssertLineIs("echo f")),
_.DownArrow));

SetHistory("echo foo", "get bar", "echo f");
Test("ec", Keys(
"ec", _.UpArrow, _.F8,
_.DownArrow, CheckThat(() => AssertLineIs("get bar")),
_.DownArrow, CheckThat(() => AssertLineIs("echo f")),
_.Shift_F3));

SetHistory("echo kv", "get bar", "echo f");
Test("e", Keys(
"e",
_.UpArrow, CheckThat(() => AssertLineIs("echo f")),
_.Ctrl_r, "v", _.Escape,
CheckThat(() => AssertLineIs("echo kv")),
_.Ctrl_s, "f", _.Escape,
CheckThat(() => AssertLineIs("echo f")),
_.UpArrow, CheckThat(() => AssertLineIs("get bar")),
_.DownArrow, _.DownArrow));
}

[SkippableFact]
Expand Down