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

#1512 Update behaviour of rl_unix_word_rubout #1592

Merged
merged 3 commits into from
Jul 1, 2016

Conversation

edi9999
Copy link
Contributor

@edi9999 edi9999 commented Jun 22, 2016

Closes #1512

Now, unix_word_rubout behaves like in bash, eg it deletes the last word in the command line.

For example , if you type

:open -t https://github.com/The-Compiler/qutebrowser/

and your cursor is at the end of the line, hitting c-w will bring you to :open -t.

If you liked the previous behaviour, you can use Ctrl-Backspace


This change is Reviewable

@edi9999 edi9999 changed the title Update behaviour of rl_unix_word_rubout #512 Update behaviour of rl_unix_word_rubout Jun 22, 2016
@edi9999 edi9999 changed the title #512 Update behaviour of rl_unix_word_rubout #1512 Update behaviour of rl_unix_word_rubout Jun 22, 2016
@codecov-io
Copy link

codecov-io commented Jun 22, 2016

Current coverage is 81.43%

Merging #1592 into master will decrease coverage by 0.02%

@@             master      #1592   diff @@
==========================================
  Files           106        106          
  Lines         15374      15386    +12   
  Methods           0          0          
  Messages          0          0          
  Branches       2463       2465     +2   
==========================================
+ Hits          12524      12530     +6   
- Misses         2382       2386     +4   
- Partials        468        470     +2   

Powered by Codecov. Last updated by 2b28574...bfeba3c

@The-Compiler
Copy link
Member

Thanks! Could you please adjust test_rl_unix_word_rubout in tests/unit/misc/test_readline.py to add a new test for this?

I also wonder if this code could be simplified a bit by using split.simple_split (in utils/split.py) and len() based on the text - what do you think?

@edi9999
Copy link
Contributor Author

edi9999 commented Jun 24, 2016

Hi, I have just added a test for the new readline behaviour.

Regarding simple_split, here is what I found out :

simple_split(':open -t ls ', True)
gives :
[':open', ' -t', ' ls', ' ']

However, since we want to remove ls (without the leading space), we would actually want to have the spaces 'appended' at the end of the words.

Here's what we could do :

simple_split(':open -t ls ', True, append_to="right")
[':open ', '-t ', 'ls ']

Do you want me to add another parameter for that ? I find the simple_split method a bit complicated to understand if I add this.

@edi9999
Copy link
Contributor Author

edi9999 commented Jun 27, 2016

What do you think of my last comments ?

@The-Compiler
Copy link
Member

Sorry for the silence, I was busy with the pytest sprint and then with my wisdom tooth being pulled... I still have some other stuff to do, but I'll try to get back to this today or tomorrow.

@The-Compiler
Copy link
Member

Gah, still didn't get to this. I hope I can take care of it later today or tomorrow.

@The-Compiler The-Compiler self-assigned this Jun 30, 2016
@The-Compiler
Copy link
Member

I tried playing with some simpler solutions based on str.rsplit but couldn't come up with anything that works correctly with all corner cases - I'll merge this as-is 😉

Thanks for the patience and the PR!

@The-Compiler The-Compiler merged commit 81c6942 into qutebrowser:master Jul 1, 2016
@edi9999
Copy link
Contributor Author

edi9999 commented Jul 2, 2016

No problem, i also don't think it was easy to achieve with str_split.

Thanks anyway!

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.

4 participants