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

keys: Implement unix-word-rubout (Ctrl-W) #54

Merged
merged 1 commit into from
Apr 27, 2021

Conversation

padawin
Copy link
Contributor

@padawin padawin commented Mar 31, 2021

unix-word-rubout erases all the characters before the cursor
until finding either a space or a BOL.

fixes #53

up.go Outdated Show resolved Hide resolved
up.go Outdated Show resolved Hide resolved
up.go Outdated Show resolved Hide resolved
up.go Outdated Show resolved Hide resolved
up_test.go Show resolved Hide resolved
up_test.go Show resolved Hide resolved
up.go Outdated Show resolved Hide resolved
up.go Show resolved Hide resolved
@padawin
Copy link
Contributor Author

padawin commented Apr 1, 2021

I am about to push a fixup commit tackling the above. Once approved, I'll rebase and bring the commits together.

@padawin padawin requested a review from akavel April 1, 2021 10:59
Copy link
Owner

@akavel akavel left a comment

Choose a reason for hiding this comment

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

Thanks for a prompt follow-up! A few more small things, other than that it looks good to me. Thanks a lot for finding the reference, and special kudos for tender loving care towards the tests! Oh, how I'd wish for every PR author to be so diligent...

up.go Show resolved Hide resolved
up.go Outdated Show resolved Hide resolved
up.go Outdated Show resolved Hide resolved
up_test.go Show resolved Hide resolved
up.go Show resolved Hide resolved
@akavel
Copy link
Owner

akavel commented Apr 2, 2021

(As a very side-note, just in a feeble attempt at stirring your interest enough to maybe implement some tiny itch for a future idea, I'm not sure if you've seen the concept for maybe delegating stuff to some thirdparty readline reenactment package. I'm personally finding it very interesting, though what holds me personally back from working on it, is that as a maintainer, I should really try again to untangle #42 first... But as I say, that's just a very side note, we're all trying to have fun here, so I appreciate your finding a way to do that by helping up already with this PR 🙂 )

@padawin
Copy link
Contributor Author

padawin commented Apr 2, 2021

Regarding your side note, it does indeed sound quite interesting. I could see to have a look deeper, in case there is anything I could do to contribute around there :-)

@akavel
Copy link
Owner

akavel commented Apr 5, 2021

Heyy @padawin ! Not hurrying or whatever, just to make sure we didn't miss something - I noticed you replied, however I don't seem to see any new commits yet; to make it clear, that's completely fine with me, and it's more than sensible that on Easter we focus on other things :) The only reason really I'm writing anything, is that I had enough cases in my career of people (myself included) forgetting to push the commit to a PR; and I really wouldn't want such a dumb accident in any case be the sole reason why this feature didn't get merged :) so, as I'm trying to say, absolutely no pressure or whatever (I'd be the last person to have right to put it), please treat it just as a friendly heads up :)

@padawin
Copy link
Contributor Author

padawin commented Apr 6, 2021

Hey @akavel, Thanks for following up! I didn't work on it due to Easter. I just attended the comments now and pushed them :-)

@padawin padawin requested a review from akavel April 6, 2021 11:01
Copy link
Owner

@akavel akavel left a comment

Choose a reason for hiding this comment

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

Thanks @padawin ! ❤️ 🚀

Originally, you mentioned you want to "bring the commits together", so I'm leaving it open for you in case you still want to tweak something. If not, please feel welcome to hit "Squash and merge" if it's available to you in the GitHub UI, or please let me know if I need to do it myself.

unix-word-rubout erases all the characters before the cursor
until finding either a space or a BOL.
@padawin
Copy link
Contributor Author

padawin commented Apr 12, 2021

I do not have the permissions to merge in your master @akavel , so I squashed my fixup commits, and you can merge the branch in master now. 🚀

@padawin padawin requested a review from akavel April 13, 2021 06:19
@padawin
Copy link
Contributor Author

padawin commented Apr 26, 2021

@akavel could you merge this PR when you have a moment?

@akavel
Copy link
Owner

akavel commented Apr 27, 2021

@padawin Uhh, HUGE thanks for reminding me, I was off the grid for some holidays and this fell through the cracks. Really sorry and again REALLY BIG thanks for your patience and persistence ❤️ ❤️ ❤️

@akavel akavel merged commit 840f23c into akavel:master Apr 27, 2021
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.

Make Ctrl-W work to erase the word before the cursor.
2 participants