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

Enhanced prepending commands to command line #7905

Merged
merged 4 commits into from Jun 23, 2021
Merged

Enhanced prepending commands to command line #7905

merged 4 commits into from Jun 23, 2021

Conversation

lapingenieur
Copy link
Contributor

@lapingenieur lapingenieur commented Apr 6, 2021

Description

There was a function called __fish_prepend_sudo which is very useful and I made some others for commands like git, nvim and so. But it is a bit messy to define myself a whole lot of functions with just a word changing between them. I just created a "super-prepending" function : __fish_prepend. There's no more need for __fish_prepend_sudo so I deleted it and changed its occurences in the other files.

This function is some lines longer than __fish_prepend_sudo because it needs to read arguments so it's obviously a little bit slower, but it doesn't affect the user experience and it has also a lot more features :

  • works with every imaginable value it gets (even with spaces)
  • permits to prepend as to remove the gotten value
  • with -p or --previous flags, you can add the last command in the history to the command line (Let __fish_prepend_sudo use the last commandline if there is no current one #7079)
  • if the first command on the line is sudo acts smartly with prepending (prepends/removes after the word sudo if the value is not sudo 👍)
  • has a small buit-in help with -h and --help

All it needs is at least one argument that isn't one of -h, --help, -p or --previous and it manages itself how to prepend or remove the value to/from the beginning of the lines. I put comments in the code so it's more readable, feel free to take a look!

TODOs:

  • Changes to fish usage are reflected in user documentation/manpages. There's no place in the doc for this function. However, fish users should know __fish_prepend replaces __fish_prepend...
  • Tests have been added for regressions fixed
  • User-visible changes noted in CHANGELOG.rst

@lapingenieur lapingenieur marked this pull request as ready for review April 6, 2021 21:32
share/functions/__fish_prepend.fish Outdated Show resolved Hide resolved
share/functions/__fish_prepend.fish Outdated Show resolved Hide resolved
share/functions/__fish_prepend.fish Outdated Show resolved Hide resolved
share/functions/__fish_prepend.fish Outdated Show resolved Hide resolved
share/functions/__fish_prepend.fish Outdated Show resolved Hide resolved
share/functions/__fish_prepend.fish Outdated Show resolved Hide resolved
share/functions/__fish_prepend_sudo.fish Show resolved Hide resolved
share/functions/__fish_prepend.fish Outdated Show resolved Hide resolved
share/functions/__fish_prepend.fish Outdated Show resolved Hide resolved
share/functions/__fish_shared_key_bindings.fish Outdated Show resolved Hide resolved
CHANGELOG.rst Outdated Show resolved Hide resolved
@lapingenieur
Copy link
Contributor Author

I finally simplified the function as @faho asked me and there's no -s|--sudo or -p|--previous flags anymore : it now doesn't borrow with sudo, and always looks for the last history item if the commandline is void.

Copy link
Contributor Author

@lapingenieur lapingenieur left a comment

Choose a reason for hiding this comment

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

I'm sorry if I made the changes the wrong way... It should be ok this time.

Copy link
Contributor Author

@lapingenieur lapingenieur left a comment

Choose a reason for hiding this comment

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

This one has not been marked as done (?)

@ridiculousfish ridiculousfish self-requested a review May 4, 2021 19:55
Copy link
Member

@ridiculousfish ridiculousfish left a comment

Choose a reason for hiding this comment

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

The function is misspelled as __fish_commandlin_prepend, also there's still a use of __fish_prepend which doesn't exist. Please squash your commits into one. Thanks!

@kidonng
Copy link
Contributor

kidonng commented May 27, 2021

Can we have a similar function for appending? We already have __fish_paginate which appends $PAGER / less, but I'd like to see a more configurable solution to append grep fzf and so on.

@zanchey zanchey added this to the fish 3.3.0 milestone Jun 20, 2021
@zanchey
Copy link
Member

zanchey commented Jun 20, 2021

This looks ready for 3.3.0?

@krobelus
Copy link
Member

No, there are some issues remaining - spurious __fish_prepend, missing regex escaping and some behavior changes to __fish_paginate which should probably be undone.

The two functions are not fully symmetrical. The "append" one ought to add to a whole job, but the "prepend" one is just for the current process. This also needs some changes. I started working on this but I'm not done yet.

@krobelus
Copy link
Member

I ended up rewriting the functions a bit, let me know if I should add rationale.
The behavior is fairly ok now. Maybe someone can check the first commit.

I almost always use this on the last/only job in a commandline, so
the semicolon is usually not needed.  We have always added it but I
prefer not dropping it: this feels cleaner because it's what you'd
type without the shortcut.
No behavior change intended.
Will use the "buffer_part" computation in the next commit.
…ss/job

With a command line like

	a | b <cursor> | c

 "commandline -C 0 --current-process" will place the cursor just left of "b".
This introduces two functions to
- toggle a process prefix, used for adding "sudo"
- add a job suffix, used for adding "&| less"

Not sure if they are very useful; we'll see.

Closes #7905
@krobelus krobelus merged commit 7c2dd69 into fish-shell:master Jun 23, 2021
@krobelus
Copy link
Member

Merging just in time for 3.3.0, thanks!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants