WIP - shorten directory relative to repo root #465

felixSchl · 2016-09-06T23:48:36Z

demo: https://asciinema.org/a/21be5t92ib26uddrs9ehuaovr

I reported this as a feature request early last year: #349.
I have since brushed up on my shell skills and feel comfortable delivering the said feature set.

What's left to do?

show relative path when in git repo
show relative path when in svn repo
show relative path when in hg repo
show relative path when in bzr repo
show relative path when in fossil repo
configure colouring of repo name (currently uses $LP_COLOR_PATH_ROOT) - input required
toggle feature ON/OFF via option - input required
how should this interact with the ordinary directory shortening? - input required

The way it's implemented it should be easy to add support for the supported VCS systems, provided they have a way to get the top-level directory... otherwise we might half to walk the file system, but I hope not.

I'd say supporting submodules as mentioned in the original issue would be a separate PR, but I personally lost interest in that feature.

felixSchl · 2016-09-07T17:58:45Z

Alright, detecting the root is implemented for all supported VCSs (afaict). I've also had to fix up the awk script to run on OSX, but I could imagine it being broken elsewhere since the + character was not escaped in the regex to mean literal +. I also took the liberty to span it across more lines to make it readable, but if you're precious of you sloc count, I can collapse them again.

dolmen · 2016-09-17T20:15:51Z

I've cherry-picked your second commit ("Fix up awk script and make readable") as it was good and independent of the rest of this PR. Please rebase your branch on master and push -f.

felixSchl · 2016-09-17T20:32:41Z

Alright, done.

dolmen

This implementation (even with my remarks applied) will break the bash with LP_ENABLE_SHORTEN_PATH=0 and PROMPT_DIRTRIM set case.

dolmen · 2016-09-17T20:19:02Z

liquidprompt

+        case "$LP_VCS_TYPE" in
+            git*)   git rev-parse --show-toplevel; ;;
+            hg)     hg root; ;;
+            svn)    svn info | grep 'Working Copy Root Path' | cut -d' ' -f5-; ;;


dolmen · 2016-09-17T20:20:26Z

liquidprompt

+# * Show the relative path from the root to "$PWD" in a different color
+_lp_shorten_vcs_path () {
+    local root x
+    root="$(


Move the root assignment inside each case.

dolmen · 2016-09-17T20:22:53Z

liquidprompt

+    )" || return
+    # trim leading and trailing whitespace, as well as a potentially trailing slash
+    root="${${${root##*[[:space:]]}%%[[:space:]]*}%/}"
+    if [ ! -z "$root" ]; then


if [[ -n "$root" ]]; then

dolmen · 2016-09-17T20:39:55Z

liquidprompt

@@ -1802,6 +1825,9 @@ _lp_set_prompt()
        # end of the prompt line: double spaces
        LP_MARK="$(_lp_sr "$(_lp_smart_mark $LP_VCS_TYPE)")"

+        _lp_shorten_path   # set LP_PWD
+        _lp_shorten_vcs_path


You are calling both lp_shorten_path and _lp_shorten_vcs_path. This is inefficient as only the result of one of them will matter.

Instead of calling both, _lp_shorten_vcs_path should delegate to _lp_shorten_path in case it does not fill LP_PWD

The call to _lp_shorten_vcs_path should be in a if [[ -n "$LP_VCS" ]] block

felixSchl · 2016-09-17T21:03:59Z

This implementation (even with my remarks applied) will break the bash with LP_ENABLE_SHORTEN_PATH=0 and PROMPT_DIRTRIM set case.

From the comments above it seems you want to do either trimming or relative dirs? What do you think about combining the two, i.e. generalising the code to trim and then apply it as a last transform on whatever is present?

dolmen · 2016-09-17T21:36:17Z

This implementation (even with my remarks applied) will break the bash with LP_ENABLE_SHORTEN_PATH=0 and PROMPT_DIRTRIM set case.
From the comments above it seems you want to do either trimming or relative dirs?

No. I'm just telling the this particular user configuration has been so optimised in liquidprompt that _lp_shorten_path is replaced by an implementation never update LP_PWD. If LP_PWD is changed by something else, nobody will restore it when exiting a VCS controlled directory.

One major concern is stability. I like your relative dirs idea (so once we have a good implementation I want it to be enabled by default), but I want existing users who upgrade to still be able to switch it off and go back to the previous behaviour.

What do you think about combining the two, i.e. generalising the code to trim and then apply it as a last transform on whatever is present?

This is interesting, but this is a much more tough project. It would require a more major rework of Liquidprompt and to be much more careful to not break things.

dolmen · 2016-09-17T21:42:11Z

liquidprompt

    # trim leading and trailing whitespace, as well as a potentially trailing slash
-    root="${${${root##*[[:space:]]}%%[[:space:]]*}%/}"
-    if [ ! -z "$root" ]; then
+    if [ -n "$root" ]; then


Use double brackets: [[ ... ]] instead of [ ... ]

felixSchl · 2016-09-17T21:43:17Z

It would require a more major rework of Liquidprompt and to be much more careful to not break things.

What is holding me back a little is the lack of automated tests that I can run my changes through. Manually verifying is quite mundane for so many scenarios. I saw that many functions change global variables and are expected to be called in certain places under certain conditions. For example, I would have expected the shortening function to take as input the path to shorten and then return the new, shortened path, which would then be set at call site. I tried to make the added vcs shorten function to follow the existing pattern.

Would you accept a PR to set up CI with say travis?

felixSchl · 2016-09-17T21:44:30Z

Btw, I am trying to reduce the number of added commits as we go along, but we can certainly squash later on once we're happy to remove meaningless commits like the one that just came in ("Use double-brackets") from the history.

dolmen · 2016-09-17T21:46:03Z

👍 for Travis. But we have no testsuite. The test.sh is very old and does not work with the current code. Open another issue to discuss that.

dolmen · 2016-09-17T21:48:01Z

Btw, I am trying to reduce the number of added commits as we go along, but we can certainly squash later on once we're happy to remove commits like the one that just came in ("Use double-brackets").

You can squash now if you want and push -f.

Rycieos · 2021-01-28T20:05:10Z

Release Candidate v2.0.0-rc.1 is now out, which means that the rework is complete. This has merge conflicts that need to be fixed.

Part of v2.0 was to support multiple path shortening methods and to highlight the VCS root and last directory. This gets close to what you were trying to do here, but none of the shortening methods show the path relative to the repo root (though all path formats will never shorten the repo root if LP_PATH_VCS_ROOT is enabled). If you want to rebase and refactor to add a shortening method that would do that, I would be happy to merge such a feature.

Docs that would be helpful:
_lp_path_format()
LP_PATH_METHOD
LP_PATH_VCS_ROOT

And the _lp_path_format() source:
https://github.com/nojhan/liquidprompt/blob/2daf0e453039950d79dcf0eec5a04931e363a92a/liquidprompt#L861-L867

And feel free to ask for help!

felixSchl force-pushed the feature/vcs-reldir branch 3 times, most recently from 03a8d80 to b9390a3 Compare September 7, 2016 17:56

felixSchl force-pushed the feature/vcs-reldir branch from b9390a3 to 406dc26 Compare September 17, 2016 20:32

dolmen requested changes Sep 17, 2016

View reviewed changes

dolmen reviewed Sep 17, 2016

View reviewed changes

Shorten directory relative to repo root

200603c

felixSchl force-pushed the feature/vcs-reldir branch from ed7935d to 200603c Compare September 17, 2016 21:50

felixSchl mentioned this pull request Sep 17, 2016

Set up test suite and CI #469

Closed

Rycieos mentioned this pull request Jan 30, 2021

Show path relative to repo #349

Closed

Rycieos mentioned this pull request Dec 5, 2022

LP_PATH_METHOD truncate till VCS root #761

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP - shorten directory relative to repo root #465

WIP - shorten directory relative to repo root #465

felixSchl commented Sep 6, 2016 •

edited

felixSchl commented Sep 7, 2016

dolmen commented Sep 17, 2016 •

edited

felixSchl commented Sep 17, 2016

dolmen left a comment

dolmen Sep 17, 2016

dolmen Sep 17, 2016

dolmen Sep 17, 2016

dolmen Sep 17, 2016

felixSchl commented Sep 17, 2016 •

edited

dolmen commented Sep 17, 2016

dolmen Sep 17, 2016

felixSchl commented Sep 17, 2016

felixSchl commented Sep 17, 2016 •

edited

dolmen commented Sep 17, 2016

dolmen commented Sep 17, 2016

Rycieos commented Jan 28, 2021

WIP - shorten directory relative to repo root #465

Are you sure you want to change the base?

WIP - shorten directory relative to repo root #465

Conversation

felixSchl commented Sep 6, 2016 • edited

felixSchl commented Sep 7, 2016

dolmen commented Sep 17, 2016 • edited

felixSchl commented Sep 17, 2016

dolmen left a comment

Choose a reason for hiding this comment

dolmen Sep 17, 2016

Choose a reason for hiding this comment

dolmen Sep 17, 2016

Choose a reason for hiding this comment

dolmen Sep 17, 2016

Choose a reason for hiding this comment

dolmen Sep 17, 2016

Choose a reason for hiding this comment

felixSchl commented Sep 17, 2016 • edited

dolmen commented Sep 17, 2016

dolmen Sep 17, 2016

Choose a reason for hiding this comment

felixSchl commented Sep 17, 2016

felixSchl commented Sep 17, 2016 • edited

dolmen commented Sep 17, 2016

dolmen commented Sep 17, 2016

Rycieos commented Jan 28, 2021

felixSchl commented Sep 6, 2016 •

edited

dolmen commented Sep 17, 2016 •

edited

felixSchl commented Sep 17, 2016 •

edited

felixSchl commented Sep 17, 2016 •

edited