Hugo Machet: 1 Implement command/search history 4 files changed, 181 insertions(+), 20 deletions(-)
Copy & paste the following snippet into your terminal to import this patchset into git:
curl -s https://lists.sr.ht/~leon_plickat/public-inbox/patches/30731/mbox | git am -3Learn more about email & git
--- * Not sure what to do with the cursor.
Just disable it. I eventually want users being able to edit history commands, but that should be done in a different patch; It's cleaner if we separate that. > * Maybe the current input stored the first time arrow up is pressed > should be deleted, not sure where without messing up the Arrays. The input should just stay in the input buffer. Maybe the user scrolls down again to that line and wants to continue editing and using it.
* Maybe the current input stored the first time arrow up is pressed should be deleted, not sure where without messing up the Arrays. * Is the ArenaAllocator really needed or we could use context.gpa?
I think it makes cleanup on exit nicer, but you can also implement it without an arena, if you prefer.
src/History.zig | 49 +++++++++++++++++++++++++++ src/UserInterface.zig | 73 +++++++++++++++++++++++++++++----------- src/mode.zig | 2 ++ src/nfm.zig | 77 ++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 181 insertions(+), 20 deletions(-) create mode 100644 src/History.zig diff --git a/src/History.zig b/src/History.zig new file mode 100644 index 000000000000..21c6ed7a2689 --- /dev/null +++ b/src/History.zig @@ -0,0 +1,49 @@ +// This file is part of nfm, the neat file manager. +// +// Copyright © 2021 - 2022 Leon Henrik Plickat
You probably want to change this to your name :D
+// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License version 3 as published +// by the Free Software Foundation. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program. If not, see <https://www.gnu.org/licenses/>. + +const std = @import("std"); +const heap = std.heap; + +const context = &@import("nfm.zig").context; + +const Self = @This(); + +arena: heap.ArenaAllocator, + +command: std.ArrayListUnmanaged([]const u8) = .{}, +search: std.ArrayListUnmanaged([]const u8) = .{}, + +/// Number of time arrow up/down have been pressed. +index: usize = 0,
I don't think this is really needed. The index in Mode.user_input should do everything this does right now.
+ +pub fn init(self: *Self) void { + self.arena = heap.ArenaAllocator.init(context.gpa); + errdefer self.arena.deinit(); +}
I think it might be cheaper to cheat and append instead of prepend the
items, because prepending needs to do a realloc and memcpy each time.
If we store the history list in reverse, we don't have to do that
and just need to calculate the reverse index on render and commit,
which is super cheap.
It would also be cool if this function would check the previous
history item and if it is the same as the current one, not add it.
That way we could easiely avoid duplicate entries. But that can happen
in a later patch.
> + if (context.mode.user_input.cmd_index != null or
> + context.mode.user_input.search_index != null)
> + {
I think these two can be unified into a single variable. There will
never be a case where a Mode.user_input instance handles multiple
UserInputOperations at the same time.
> + const prefix = @tagName(context.mode.user_input.operation);
> + const prefix_len = prefix.len + 4;
> + debug.assert(prefix_len < min_width);
You have drawing the prefix in both branches right now. Probably
better to put it before the if.
> + if (rest > 0) try self.term.writeByteNTimes(' ', rest);
This is not needed here. Drawing spaces for the "rest" is only used
when drawing a sold line, like the title bar or the cursor. It is
needed there to get the colour on screen, but if the line is empty
with the default background, it is not needed.
+ +pub fn deinit(self: *Self) void { + self.command.deinit(self.arena.allocator()); + self.search.deinit(self.arena.allocator()); + self.arena.deinit(); +} + +pub fn prepend(self: *Self, op: enum { command, search }, item: []u8) !void { + const alloc = self.arena.allocator(); + switch (op) { + .command => try self.command.insert(alloc, 0, try alloc.dupe(u8, item)), + .search => try self.search.insert(alloc, 0, try alloc.dupe(u8, item)), + } +} diff --git a/src/UserInterface.zig b/src/UserInterface.zig index f130247ae6c0..bcb9f9b91df9 100644 --- a/src/UserInterface.zig +++ b/src/UserInterface.zig @@ -281,28 +281,63 @@ fn drawTitle(self: *Self, title: []const u8) !void { } else { try self.term.hideCursor(); } + switch (context.mode) { .user_input => { - const prefix = @tagName(context.mode.user_input.operation); - const prefix_len = prefix.len + 4; - debug.assert(prefix_len < min_width); - - try self.term.setAttribute(.{ .bg = .blue, .fg = .black, .bold = true }); - try self.term.writeByte(' '); - try self.term.writeAll(prefix); - try self.term.writeByte(':'); - try self.term.writeByte(' '); - try self.term.setAttribute(.{ .bg = .black, .fg = .white }); - try self.term.writeByte(' '); - try self.term.moveCursorTo(0, prefix_len); - // TODO input line scrolling - const rest = try self.term.writeLine( - self.term.width - prefix_len, - context.mode.user_input.buffer.buffer.items, - ); - if (rest > 0) try self.term.writeByteNTimes(' ', rest); + if (context.mode.user_input.cmd_index != null or + context.mode.user_input.search_index != null) + { + const prefix = @tagName(context.mode.user_input.operation); + const prefix_len = prefix.len + 4; + debug.assert(prefix_len < min_width); + + try self.term.setAttribute(.{ .bg = .blue, .fg = .black, .bold = true }); + try self.term.writeByte(' '); + try self.term.writeAll(prefix); + try self.term.writeByte(':'); + try self.term.writeByte(' '); + try self.term.setAttribute(.{ .bg = .black, .fg = .magenta, .italic = true }); + try self.term.writeByte(' '); + try self.term.moveCursorTo(0, prefix_len); + // TODO input line scrolling + const rest = blk: { + if (context.mode.user_input.cmd_index) |i| { + break :blk try self.term.writeLine( + self.term.width - prefix_len, + context.history.command.items[i], + ); + } + if (context.mode.user_input.search_index) |i| { + break :blk try self.term.writeLine( + self.term.width - prefix_len, + context.history.search.items[i], + ); + } + unreachable; + }; + if (rest > 0) try self.term.writeByteNTimes(' ', rest); + } else { + const prefix = @tagName(context.mode.user_input.operation); + const prefix_len = prefix.len + 4; + debug.assert(prefix_len < min_width); + + try self.term.setAttribute(.{ .bg = .blue, .fg = .black, .bold = true }); + try self.term.writeByte(' '); + try self.term.writeAll(prefix); + try self.term.writeByte(':'); + try self.term.writeByte(' '); + try self.term.setAttribute(.{ .bg = .black, .fg = .white }); + try self.term.writeByte(' '); + try self.term.moveCursorTo(0, prefix_len); + // TODO input line scrolling + const rest = try self.term.writeLine( + self.term.width - prefix_len, + context.mode.user_input.buffer.buffer.items, + ); + if (rest > 0) try self.term.writeByteNTimes(' ', rest); - try self.term.moveCursorTo(0, prefix_len + context.mode.user_input.buffer.cursor); + try self.term.moveCursorTo(0, prefix_len + context.mode.user_input.buffer.cursor); + } },
Same as above, these can probably be a single variable.
> + .arrow_up => {
> + switch (context.mode.user_input.operation) {
> + .command => {
If there only is a single index, this switch won't be necessary
anymore.
> + try context.history.prepend(
> + .command,
> + context.mode.user_input.buffer.buffer.items,
> + );
> + }
Instead of adding the current input to the history, just leave it in
the InputBuffer. That's why the history index is suppoed to be a
?usize. null means user_input, value means history item.
> + context.mode.setMessage(.info, "Beginning of command history");
I don't think these messages should be here. They block the view of
the titlebar, where the user is currently looking for content. So
just stop scrolling.
---
Nice patch overall, thanks for working on this!
Friendly greetings,
Leon Henrik Plickat
.message => { try self.term.setAttribute(context.mode.message.level.getAttribute()); diff --git a/src/mode.zig b/src/mode.zig index b0b0fe965b52..36c07b592930 100644 --- a/src/mode.zig +++ b/src/mode.zig @@ -63,6 +63,8 @@ pub const Mode = union(enum) { const UserInput = @This(); operation: UserInputOperation, buffer: InputBuffer, + cmd_index: ?usize = null, + search_index: ?usize = null, }, pub fn setNav(self: *Self) void { diff --git a/src/nfm.zig b/src/nfm.zig index f06e0d2ff592..0be249dc2aa3 100644 --- a/src/nfm.zig +++ b/src/nfm.zig @@ -33,6 +33,7 @@ const log = std.log; const CommandTokenizer = @import("CommandTokenizer.zig"); const Config = @import("Config.zig"); const DirMap = @import("DirMap.zig"); +const History = @import("History.zig"); const Mode = @import("mode.zig").Mode; const UserInterface = @import("UserInterface.zig"); const util = @import("util.zig"); @@ -58,6 +59,7 @@ pub const Context = struct { cwd: *DirMap.Dir = undefined, // TODO should this be an attribute of DirMap? initial_cwd_path: []const u8 = undefined, search: ?Regex = null, + history: History = undefined, show_hidden: bool = false, time_of_last_mark_clear: ?os.timespec = null, @@ -328,6 +330,9 @@ fn nfm() !void { defer context.ui.deinit(); try context.ui.start(); + context.history.init(); + defer context.history.deinit(); + defer { if (context.search) |*search| { search.deinit(); @@ -399,6 +404,7 @@ fn handleEventUserInput(ev: spoon.Event) !void { .ascii => |key| switch (key) { '\n', '\r' => { try handleReturnUserInput(); + context.history.index = 0; return; }, 127 => buffer.delete(.left, 1), // Backspace @@ -445,9 +451,76 @@ fn handleEventUserInput(ev: spoon.Event) !void { }, else => return, }, - .escape => context.mode.setNav(), + .escape => { + context.history.index = 0; + context.mode.setNav(); + }, .arrow_left => buffer.moveCursor(.left, 1), .arrow_right => buffer.moveCursor(.right, 1), + .arrow_up => { + switch (context.mode.user_input.operation) { + .command => { + if (context.history.index == 0) { + try context.history.prepend( + .command, + context.mode.user_input.buffer.buffer.items, + ); + } + + context.history.index += 1; + + if (context.history.index >= context.history.command.items.len) { + context.mode.setMessage(.info, "Beginning of command history"); + return; + } + + context.mode.user_input.cmd_index = context.history.index; + }, + .search => { + if (context.history.index == 0) { + try context.history.prepend( + .search, + context.mode.user_input.buffer.buffer.items, + ); + } + + context.history.index += 1; + + if (context.history.index >= context.history.search.items.len) { + context.mode.setMessage(.info, "Beginning of search history"); + return; + } + + context.mode.user_input.search_index = context.history.index; + }, + else => {}, + } + }, + .arrow_down => { + switch (context.mode.user_input.operation) { + .command => { + context.history.index -= 1; + + if (context.history.index == 0) { + context.mode.setMessage(.info, "End of command history"); + return; + } + + context.mode.user_input.cmd_index = context.history.index; + }, + .search => { + context.history.index -= 1; + + if (context.history.index == 0) { + context.mode.setMessage(.info, "End of search history"); + return; + } + + context.mode.user_input.search_index = context.history.index; + }, + else => {}, + } + }, .delete => buffer.delete(.right, 1), else => return, } @@ -465,6 +538,7 @@ fn handleReturnUserInput() !void { return; } const cmd = try user_input.buffer.buffer.toOwnedSliceSentinel(0); + try context.history.prepend(.command, cmd); defer context.gpa.free(cmd); try context.run(cmd, "/bin/sh or subprocess"); }, @@ -478,6 +552,7 @@ fn handleReturnUserInput() !void { context.mode.setMessage(.err, "Invalid regex"); return; }; + try context.history.prepend(.search, user_input.buffer.buffer.items); // This function will never need this parameter, so just input // garbage to make the compiler happy. -- 2.35.1