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

Add ability to customize the amount of path shortening in prompt_pwd #2473

Closed
wants to merge 3 commits into from

Conversation

garyp
Copy link
Contributor

@garyp garyp commented Oct 9, 2015

Allows the length of each shortened path component to be customized by setting the fish_prompt_pwd_abbr variable to the number of extra characters to include. Maintains the default behavior of shortening path components to just one character. You can also set fish_prompt_pwd_abbr to the empty value to disable shortening completely.

Allows the length of each shortened path component to be customized by setting the `fish_prompt_pwd_abbr` variable to the number of extra characters to include. Maintains the default behavior of shortening path components to just one character. You can also set `fish_prompt_pwd_abbr` to the empty value to disable shortening completely.
@faho
Copy link
Member

faho commented Oct 9, 2015

I see that people want this (I can't say I'm in love with one-char-per-dir), but I'm not sure about this solution.

I think of functions as the better customization point - the reasoning being that you can actually add comments to them, and have more space to explain what it does in general. If I want to change how the pwd is printed in my prompt, I type either type prompt<TAB> or type fish_prompt (depending on if I've seen that "fish_prompt" is responsible for printing the prompt or not) - either way, I end up looking at prompt_pwd. And there I see how it shortens the prompt, and can then run funced prompt_pwd and funcsave prompt_pwd, and I'm done. This example by itself isn't too useful (since I'd also be staring the variable right in its data-holding face, in true technicolor), but it's rather consistent in pretty much the entirety of fish.

However, I'm not completely opposed, but I have three reservations regarding the finer details of the implementation too:

  1. This always writes a universal variable, which I'm not particularly fond of - why not just check if it's defined and if not define it locally? (I.e. move the check inside of the function and do "set -l" instead of "set -U"
  2. The variable contains the number of extra characters, which is a bit counter-intuitive. I'd like it more if it contained the complete number of characters.

I've been trying the following match-regex:

(\.{0,1})([^/]{0,'"$fish_prompt_pwd_abbr"'})[^/]*/

which will print up to fish_prompt_pwd_abbr plus a leading dot (if any exists). The issue being that it completely obliterates all directories when set to 0, which doesn't seem useful. (Of course an explicit check could be added that sets it to 1 locally if it's set to 0)

\3. Can anyone think of a better name for the variable? "fish_prompt_pwd_abbr" sounds like an abbreviation for the prompt or a variable that turns abbreviation on. It doesn't need to be "fish_number_of_characters_per_directory_in_prompt_pwd", but maybe fish_prompt_pwd_abbrchars or so?

(Markdowns numbered lists are kind of annoying)

There might be other things to do with prompt_pwd - maybe only shorten if a directory is more than a certain number of characters and then mark it (with a nice unicode "…" or so), or actually ellipsize it (i.e. cut in the middle). Or make the pwd actually fill the prompt up to some number of characters (though this would need a bit of re-architecting).

@faho faho self-assigned this Oct 13, 2015
@faho
Copy link
Member

faho commented Oct 23, 2015

@garyp: Ping? Do you want to work on my criticisms or should I?

@garyp
Copy link
Contributor Author

garyp commented Oct 23, 2015

I think of functions as the better customization point - the reasoning being that you can actually add comments to them, and have more space to explain what it does in general.

Yeah, there are definitely many prompt customizations that people might want and it's hard to know where to draw the line between customization that can be accomplished via knobs such as this one, and customization that requires the user to override the whole prompt_pwd function. When making only tiny changes to the functionality, it's annoying to have to track the upstream version of the function to pull in improvements/updates (e.g. as prompt_pwd has recently received).

I'd like to think that changing the number of characters is going to be a common-enough need that it should be supported within the default prompt_pwd function. But I understand this can be a slippery slope.

There is precedent for non-function customization within fish by using universal variables: they're used for configuring colors. However if there's a different preferred mechanism for exposing knobs to users, I can update this pull request to use it instead.

This always writes a universal variable, which I'm not particularly fond of - why not just check if it's defined and if not define it locally? (I.e. move the check inside of the function and do "set -l" instead of "set -U"

Makes sense. I'll make this change.

The variable contains the number of extra characters, which is a bit counter-intuitive. I'd like it more if it contained the complete number of characters.

I've been trying the following match-regex:

(.{0,1})([^/]{0,'"$fish_prompt_pwd_abbr"'})[^/]*/
which will print up to fish_prompt_pwd_abbr plus a leading dot (if any exists). The issue being that it completely obliterates all directories when set to 0, which doesn't seem useful. (Of course an explicit check could be added that sets it to 1 locally if it's set to 0)

Agreed. I also wasn't too happy with having it represent extra characters instead of total characters. I'll try to come up with an expression that would allow fish_prompt_pwd_abbr to represent total characters. I'm trying to avoid using the math function, which would otherwise make this easier, so that prompt_pwd doesn't involve shelling out to external programs (since math shells out to bc).

\3. Can anyone think of a better name for the variable? "fish_prompt_pwd_abbr" sounds like an abbreviation for the prompt or a variable that turns abbreviation on. It doesn't need to be "fish_number_of_characters_per_directory_in_prompt_pwd", but maybe fish_prompt_pwd_abbrchars or so?

How about fish_prompt_pwd_dir_length?

There might be other things to do with prompt_pwd - maybe only shorten if a directory is more than a certain number of characters and then mark it (with a nice unicode "…" or so), or actually ellipsize it (i.e. cut in the middle). Or make the pwd actually fill the prompt up to some number of characters (though this would need a bit of re-architecting).

I actually really like the idea of adding a Unicode ellipsis to indicate shortened paths. I'll add it as default behavior in this pull request unless you think that it warrants a separate discussion.

@faho
Copy link
Member

faho commented Oct 23, 2015

When making only tiny changes to the functionality, it's annoying to have to track the upstream version of the function to pull in improvements/updates (e.g. as prompt_pwd has recently received).

Yeah, though I do think in this particular case prompt_pwd should be reasonably stable - I don't anticipate many more changes.

But I understand this can be a slippery slope.

I don't generally buy the idea of the slippery slope - we're capable of evaluating every step on its own, and we're capable of saying "stop".

There is precedent for non-function customization within fish by using universal variables: they're used for configuring colors.

That's not quite the same thing - the colors are used all over the place, even in C++ code (AFAIK).

Anyway, I've thought about this, and I think using variables is okay - though I've also thought about allowing it as an argument to prompt_pwd - this would allow prompts to override it (though I'm not too sure how useful that is).

So what I'd say is:

  • If there's an argument to prompt_pwd, use that
  • If a variable is defined, use that
  • Else, use "1" as the default value

Agreed. I also wasn't too happy with having it represent extra characters instead of total characters. I'll try to come up with an expression that would allow fish_prompt_pwd_abbr to represent total characters.

I've also worked on this a bit, and what I've come up with is:

function prompt_pwd --description 'Print the current working directory, shortened to fit the prompt'
    set realhome ~
    # This allows overriding fish_prompt_pwd_abbr from the outside (global or universal) without leaking it
    set -q fish_prompt_pwd_abbr; or set -l fish_prompt_pwd_abbr 1
    # An unfortunate technicality means a value of 0 would shorten the prompt to (e.g.) //////lastdirectory - disable shortening instead
    [ $fish_prompt_pwd_abbr -eq 0 ]; and set -l fish_prompt_pwd_abbr 1
    # Replace $HOME with "~"
    set -l tmp (string replace -r '^'"$realhome"'($|/)' '~$1' $PWD)
    # Shorten to at most $fish_prompt_pwd_abbr characters per directory
    string replace -ar '(\.{0,1})([^/]{0,'"$fish_prompt_pwd_abbr"'})[^/]*/' '$1$2/' -- $tmp
end

which does just that.

How about fish_prompt_pwd_dir_length?

Sounds groovy! (I've been working on my adjectives)

I actually really like the idea of adding a Unicode ellipsis to indicate shortened paths. I'll add it as default behavior in this pull request unless you think that it warrants a separate discussion.

Humm.... I'll have to check how well we're currently doing with non-Unicode locales. If we're usually doing great, this would break it more (so it'd need a fallback if we're not unicode-capable). Otherwise it'd be nice as default.

@garyp
Copy link
Contributor Author

garyp commented Oct 24, 2015

I actually really like the idea of adding a Unicode ellipsis to indicate shortened paths. I'll add it as default behavior in this pull request unless you think that it warrants a separate discussion.

Humm.... I'll have to check how well we're currently doing with non-Unicode locales. If we're usually doing great, this would break it more (so it'd need a fallback if we're not unicode-capable). Otherwise it'd be nice as default.

Oh yes. I've gotten too used to having Unicode support everywhere I care about. I'll leave out the Unicode ellipsis then unless there's a recommended way of testing for Unicode support from a fish function?

@garyp
Copy link
Contributor Author

garyp commented Oct 24, 2015

@faho let me know what you think of the updated version. I wasn't sure if your previous comment was a request to add support for a prompt_pwd argument or just a suggestion? I also simplified the regex a bit and made fish_prompt_pwd_dir_length -eq 0 disable abbreviation.

I'm not familiar with the code review process the fish-shell project uses. Do you prefer that I squash my commits as I go or do it at the end? Also wasn't sure what the whitespace rules are. The CONTRIBUTING.md file says all spaces for C code, but the shell code seems to use tabs more often than spaces.

@faho
Copy link
Member

faho commented Oct 24, 2015

Do you prefer that I squash my commits as I go or do it at the end?

I'll do it at the end.

Also wasn't sure what the whitespace rules are. The CONTRIBUTING.md file says all spaces for C code, but the shell code seems to use tabs more often than spaces.

We do whatever fish_indent does - though it's not perfect (It doesn't currently allow COMMAND; and OTHERCOMMAND on one line). But as long as it's readable, I don't care much.

@faho
Copy link
Member

faho commented Oct 24, 2015

I've now tested this a bit and it looks good - it also disables shortening for invalid values, which is nice.

The last thing I've been thinking about is how to document this. I mean it should be MENTIONED IN THE RELEASE NOTES anyway, but I'm not sure if we don't want to add a section to the docs - maybe a "Useful functions for writing your own prompt" section like we have for completions, or a complete section on special variables (or customization points in general)?

Of course that can be done later - I'll add a label to remind me (or anyone else looking to write documentation).

@garyp: Is there anything else you'd like to do or can I merge this?

@faho faho added the docs An issue/PR that touches or should touch the docs label Oct 24, 2015
@garyp
Copy link
Contributor Author

garyp commented Oct 25, 2015

@faho I'm all done. Feel free to squash & merge.

@zanchey
Copy link
Member

zanchey commented Oct 26, 2015

hould be MENTIONED IN THE RELEASE NOTES anyway

@faho, OK, OK, shall we make a separate tag?

@faho faho added the release notes Something that is or should be mentioned in the release notes label Oct 26, 2015
@faho
Copy link
Member

faho commented Oct 26, 2015

@zanchey: Made the label "releasenotes". Now to apply it.

@faho
Copy link
Member

faho commented Oct 26, 2015

Merged as 09bd938. Thanks!

@faho faho closed this Oct 26, 2015
@faho faho added this to the next-2.x milestone Oct 26, 2015
0rax added a commit to 0rax/fishline that referenced this pull request May 15, 2016
- VIMODE: remove force delete of fish_mode_prompt as it was shown the
  first time nonetheless. Will require the user to deactivate the
  default fish_mode_promopt by creating an empty function
- PWD: now uses prompt_pwd instead of the path directly as some
  modification will be added to this function alowing the user to set
  the abbreviation length (fish-shell/fish-shell#2473)
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
docs An issue/PR that touches or should touch the docs release notes Something that is or should be mentioned in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants