Readline movement support

[jonhiggs]

It would be great if up would support my Readline movement commands.

https://www.gnu.org/software/bash/manual/html_node/Bindable-Readline-Commands.html

Thanks.

[akavel]

I agree; but I don't plan to do this myself as of now, as I don't have enough resources for that, unfortunately.

If someone wants to jump in and help, I'm super happy to provide mentoring, design discussion and guidance! ❤️ The process will be easiest for us both if you start talking to me here even before you start working on this. Also it's ok to fail - I won't be regretful if you don't complete the task even after we've put a lot of our time into it. I am more than sure we both will learn a lot from this anyway!

[cmacrae]

I'd love to see this! Got started and managed to get the basics implemented:

beginning-of-line (C-a)
end-of-line (C-e)
forward-char (C-f)
backward-char (C-b)

cmacrae/up@a760a13 🕺
So, now I've just got to work out how to implement the slightly harder stuff 🤔

[cmacrae]

Next I'd be looking to implement:

forward-word (M-f)
backward-word (M-b)

Curious if others interested in this feature would also like to see kill-to-end-of-line (C-k)?

[cmacrae]

Well, I've implemented kill-to-end-of-line (C-k) for anyone who wants to try it out: cmacrae/up@edeaabe

[cmacrae]

Aaaaaand with C-k comes C-y: cmacrae/up@e5cc931
This adds the ability to "yank" the contents you last killed back into the buffer at the cursor, as those used to "kill" and "yank" would expect.

I settled on killspace being the name for killed runes to live, as killring, as some may expect, doesn't really convey what it is: a single buffer space with only one potential value, not a ring/collection of previously killed runes.

One note (I don't have time to address this just at the moment, burgers and beer are calling 🍔 🍺 ):
I need to implement protection against killing an empty buffer of runes overwriting your previous killspace, so you don't lose previous kills if you C-k again when there's nothing there

[cmacrae]

Oh, and before I forget, I had another idea for a feature that I feel fits in nicely here: the ability to "navigate" to previously run pipelines. Much like ⬆️ and ⬇️ (or C-p & C-n) in a shell allow you to get to your previously run command, it'd be nice to have in up.

For instance: I have some JSON I'm forming a jq pipeline around. I'm happily chipping away through the data to get what I want, and in my next Return I make a mistake. Now, my output in the buffer (the JSON I'd been narrowing down) has been replaced by jq's error output.

Rather than having to then navigate back through my pipeline with the cursor and manually deleting the bad part of the pipeline to get back to where I was, I could just hit ⬆️ (or C-p) to get my previous pipeline, hit Return, and I'm right back where I want to be to re-think my next step.

Hope that makes sense?

[akavel]

The idea for "history navigation" sounds interesting, and I feel like it could maybe actually kinda accidentally solve #4?... (!) Maybe the results from a few old runs could also be stored gzipped in memory?... just a random thought, not necessarily a good idea.

(part of the comment was hidden)

Re: ctrlKey() vs key()

I don't understand key vs ctrlKey either, esp. for all the KeyCtrlXYZ keys, and for me it was the same that only one of them works, but I prefer to just list them both to be "better safe than sorry".

Re: killspace overwritten by insert()

About e.value[:e.cursor:e.cursor], you can see Go specification, subsection "Full slice expressions" (scroll down from the link), and compare this with details on how append() works. Changing e.value's cap would ensure that any future append(e.value) would allocate new underlying buffer, avoiding overriding killspace. If you feel the first approach is more readable, then that would make it the preferred choice here. Actually now that I look at it, the first one seems indeed more explicit/straightforward about the intention.

Re: O(n^2) in yank

Do you know the "big O" notation? That's the first question I need to ask, to know how to try answering you, as your "don't understand" here is very broad, so not sure what exactly you do vs. do not understand, sorry :)

Trying to explain quickly:

• e.killspace contains many bytes — specifically: many1 = len(e.killspace);
• yank() has a for loop, where it calls insert() many1 times;
• then, in insert(), there's a copy(e.value[...], e.value[...]), which will copy many2 bytes (where many2 = ~len(e.value) ) — in other words, a "copy byte in memory" CPU operation (let's call it "MOV" to simplify) will be called many2 times.

Given that yank() calls insert() many1 times, then each insert() calls MOV many2 times, it means that in grand scheme of things, yank() will call MOV many1 * many2 times, a.k.a. many ^ 2 times, a.k.a. O(n^2), a.k.a. Shlemiel the Painter's Algorithm. We can do better here, there's no need to use the for loop. I can show you how if you want, or try to give you some more hints, or you can still try to find it yourself. I don't want to spoil too much fun of discovery for you here too fast ;) And by the way, we could actually just make insert() take a slice for that, but varargs will be much prettier and more appropriate here.

[akavel]

@cmacrae I didn't want to risk losing your contribution to neglect, so I took the liberty to merge your branch now, plus the tweaks/fixes I suggested. Sorry for not being more patient :D and thank you so much! ❤️ I've released a v0.3.2 with those improvements. Also, I will now hide some comments in the thread which I feel are not very important for the main discussion here.

[cmacrae]

Thanks so much @akavel! I've been very busy over the last couple weeks, and a few busy weeks ahead of me. So I may be a bit quiet around this, but I hope to get some time over the holidays to tackle some of the points we've discussed 🤗

[dufferzafar]

Has there been any update on this? Would really love readline movements like Ctrl + W to delete a word.

[akavel]

@dufferzafar Could you try and add a prioritized list of what specific readline commands are the ones you miss the most? There's a lot of them, some may be tricky, so I don't expect all of them will be ever added to up; if you could list the ones you'd love most, me or someone else would have a better idea of what to focus on when trying to help you!

[dufferzafar]

Okay, I'll keep commenting here as and when use up and miss a binding.

For now, it's just ctrl+w

[cmacrae]

Oh hey! That’s so strange, I was actually thinking about this just yesterday and recalling the other parts I wanted to implement!

I’d love to get the stuff I mentioned above in early comments implemented, so I can perhaps take a look at getting all this rolling again 😊

[akavel]

@cmacrae ❤️ Just one thing to note: I started testing Ctrl-W in bash (I never used this shortcut before), and from what I see, it seems somewhat tricky. It would be awesome if you guys could try and collect a corpus of test cases, with expected input & output, to verify it does what we want... I wouldn't see much sense in adding a Ctrl-W that would work very differently than in bash/readline... it's very much OK if you just collect them and not write actual unit tests, I will then try to add the actual tests when merging. For example, assuming | marks cursor, IIUC sample tricky cases could look like:

old: `hello  my   |world`
new: `hello  |world`

old: `hello  my|   world`
new: `hello  |   world`

Though if you wish to add actual tests code too, you're certainly more than welcome! :) you can take a look at the existing (single.... :/) test, for some inspiration, if you like!

[cmacrae]

Hey @akavel! 👋 Yep, definitely agree it's important we accurately recreate any expected behaviour. I'll see what I can do in ways of testing/ensuring consistency.

One thing that could work is using this lib: https://github.com/chzyer/readline
I'm going to try and plug it in this morning and have a play around. If that works, it seems a lot of this will be solved for us!

[dufferzafar]

Wow, integrating with full fledged Readline lib would be great!

[dwurf]

Wow, integrating with full fledged Readline lib would be great!

Agreed. I use readline vi mode so it's really jarring switching between vi keys in bash/python/sqlite/... and emacs keys in up!

[NightMachinery]

I also think that it should be possible to just integrate with the tested-and-true readline library. Another idea is to somehow integrate it with a running neovim (I think this too should be possible, but readline still seems the more promising approach to me.)

[saulrh]

Hilariously, this support is just good enough that I keep finding myself hitting C-d to delete the character at the point... which up interprets as eof so it boots me out with a completed command printed to the terminal. Means I can't use up in its intended role, since it keeps losing the stream I wanted to work on, and I have to go back to shoving it into something in /tmp anyway!

[akavel]

Hilariously, this support is just good enough that I keep finding myself hitting C-d to delete the character at the point... which up interprets as eof so it boots me out with a completed command printed to the terminal. Means I can't use up in its intended role, since it keeps losing the stream I wanted to work on, and I have to go back to shoving it into something in /tmp anyway!

@saulrh Hmmm; but where does the C-d as non-EOF come from? Is it an Emacs thing? From my experience, in bash the C-d is typically interpreted as EOF/logout, so I'm sincerely quite surprised it can be treated as something else. For example, personally I'm quite wary of accidentally hitting C-d on a Linux machine now. But I'm no expert at readline, so I wonder where and how it is also used such that C-d means delete?

As for UP, given the above, I think the only reasonable way around this would be to allow full customization of keys in the app; but that's not something I would expect to do especially soon, seeing how I have something of a writer's block with regards to UP now, unfortunately 😢

That said, for your personal purposes, you should be fine with just downloading the source (no need to even git clone at this point, it's still a single Go file as of today!), removing the ~2 lines related to C-d, and compiling it with go build up.go. Would that work for you @saulrh ?

[saulrh]

Hmmm; but where does the C-d as non-EOF come from?

It's a thing that's common to readline and emacs, yeah. bash and company interpret it as EOF when it's the only thing on a line, but when the line isn't empty it's interpreted as "delete character at point". Not sure what the first system to use it like that was.

And agree, this would probably require either a) a real customization system or b) replacing the homegrown line-editor with a "real editor", something like readline or nvim --embed.

Would that work for you @saulrh ?

It would. Thanks for the suggestion!