Add SPACESHIP_DIR_TRUNC_PREFIX, a prefix before cwd when truncated #281

segevfiner · 2017-12-15T13:20:35Z

A version of #280 for the 3.0 branch

When the cwd is truncated by SPACESHIP_DIR_TRUNC instead of showing:

a/b/c

It will show:

.../a/b/c

The prefix is configurable of course and can be disabled by setting it to an empty string.

I also modified SPACESHIP_DIR_TRUNC_REPO to use SPACESHIP_DIR_TRUNC_PREFIX and made it not truncate for a repository that is below the root directory. In addition, I optimized its code to use Zsh constructs instead of command substitutions.

cc @maximbaz

Fixes #343

denysdovhan · 2017-12-18T10:18:17Z

Thanks for PR. Let's do not include this feature in 3.0. I just don't want to make next major release too complex, because it already takes too much time to make it ready. We can release this later as a minor release after 3.0.

maximbaz · 2018-01-14T12:14:39Z

sections/dir.zsh

@@ -11,6 +11,7 @@ SPACESHIP_DIR_SHOW="${SPACESHIP_DIR_SHOW:=true}"
 SPACESHIP_DIR_PREFIX="${SPACESHIP_DIR_PREFIX:="in "}"
 SPACESHIP_DIR_SUFFIX="${SPACESHIP_DIR_SUFFIX:="$SPACESHIP_PROMPT_DEFAULT_SUFFIX"}"
 SPACESHIP_DIR_TRUNC="${SPACESHIP_DIR_TRUNC:=3}"
+SPACESHIP_DIR_TRUNC_PREFIX="${SPACESHIP_DIR_TRUNC_PREFIX:=.../}"


Just not to forget, when you rebase on latest 3.0 this line should become SPACESHIP_DIR_TRUNC_PREFIX="${SPACESHIP_DIR_TRUNC_PREFIX=".../"}"

maximbaz

I've been using this patch since it was created and I love it!

salmanulfarzy · 2018-01-22T15:47:53Z

Fixes #343

salmanulfarzy

Looks good. Would be great with some comments.

salmanulfarzy · 2018-02-28T12:08:15Z

sections/dir.zsh

-    dir="$git_root:t${$(expr $(pwd) : "$git_root\(.*\)")}"
+
+    if [[ $git_root:h == / ]]; then
+      trunc_prefix=/


If I understand correctly, This checks whether the git_root is /, Right ? But why would someone keep / under git !! 😕

Or did I miss something ?

This checks whether the parent of git_root is /. For strange people who keep their repos directly under the root (I'm not doing that personally).

Sorry I missed that :h modifier. Not judging anyone, Still it feels kinda weird to store repositories directly under root.

salmanulfarzy · 2018-02-28T12:14:30Z

sections/dir.zsh

+      trunc_prefix=$SPACESHIP_DIR_TRUNC_PREFIX
+    fi
+
+    dir="$trunc_prefix$git_root:t${PWD#$~~git_root}"


There is sorcery 🦄, Would be great if you could leave some comments.

salmanulfarzy

That looks wonderful @segevfiner 👏

salmanulfarzy

Looks like #385 affects this too. Could you please take a look @segevfiner ?

salmanulfarzy · 2018-03-04T13:56:40Z

sections/dir.zsh

+    # `$~~` avoids `GLOB_SUBST` so that `$git_root` won't actually be
+    # considered a pattern and matched literally, even if someone turns that on.
+    # See "Parameter Expansion" under the Zsh manual.
+    dir="$trunc_prefix$git_root:t${PWD#$~~git_root}"


This causes symlinked directories to appear twice.

This is because $git_root has symlinks resolved. Guess I can use the working directory with symlinks resolved here. Hopefully that's the desired behavior.

salmanulfarzy

Awesome, Hope we can merge this long standing PR soon with one more approval.

Thank you for quick response 👍

maximbaz

I already approved this earlier, looks good 👍

segevfiner · 2018-03-09T17:53:42Z

Re-based due to #385. It was fixed differently here by 62d654c.

denysdovhan · 2018-03-09T17:59:30Z

Sorry I didn't pay attention to this PR for a long time.

My point on this: I don't like the idea of cluttering prompt with .../. I'd like to have it disabled by default. Prompt looks much cleaner without it.

BTW, instead of using ..., we can try …, which is much cleaner and shorter.

denysdovhan

Please, consider my comments.

segevfiner · 2018-03-09T19:28:22Z

@denysdovhan I wasn't sure originally whether to use the Unicode ellipsis (…), but since Spaceship already uses other exotic Unicode chars and the Unicode ellipsis is quite older than them it should be OK.

In terms of whether to enable this by default or not, I guess that's up to you. Maybe @maximbaz @salmanulfarzy want to pitch in?

The facility to disable it is already there, you simply set SPACESHIP_DIR_TRUNC_PREFIX to be empty:

SPACESHIP_DIR_TRUNC_PREFIX=""

I added some more documentation so that even if you choose to disable it the user still has examples for how to enable it, or on the other hand, disable it.

maximbaz · 2018-03-09T19:43:47Z

In terms of whether to enable this by default or not, I guess that's up to you. Maybe @maximbaz @salmanulfarzy want to pitch in?

Whatever gets this PR finally merged 🙂 I personally find this feature very intuitive and I use it, but I don't mind enabling it locally if it's disabled by default.

salmanulfarzy · 2018-03-09T20:09:03Z

I prefer it disabled by default for git directories, giving a cleaner look. But I find prefix to be much useful when current path is longer than SPACESHIP_DIR_TRUC on non-git directories.

segevfiner · 2018-03-10T16:11:26Z

I prefer it disabled by default for git directories, giving a cleaner look. But I find prefix to be much useful when current path is longer than SPACESHIP_DIR_TRUC on non-git directories.

There is currently only an option to enable or disable it for both outside and inside a repo. I guess it can be split into two separate options, though I think that makes is slightly more complex for users to enable/disable.

I will start at least by setting it to disabled by default, since that appears to be the consensus. Hopefully the instructions I left on how to enable it are enough.

salmanulfarzy

Please resolve the conflict.

salmanulfarzy

👍

slhck · 2018-05-03T13:58:24Z

Just wanted to ask if there are any updates on this or anything blocking the PR from being merged?

salmanulfarzy · 2018-05-03T18:50:48Z

@slhck

Just wanted to ask if there are any updates on this or anything blocking the PR from being merged?

Maintainers were on separate pages about whether this should be enabled by default and what symbol should be used as prefix.
There are no automated tests for spaceship-prompt now. So, without extensive testing this may break some prompts and we don't want to introduce any breaking changes.

salmanulfarzy · 2018-05-03T19:04:01Z

sections/dir.zsh

@@ -21,14 +22,36 @@ SPACESHIP_DIR_COLOR="${SPACESHIP_DIR_COLOR="cyan"}"
 spaceship_dir() {
  [[ $SPACESHIP_DIR_SHOW == false ]] && return

-  local dir
+  local dir trunc_prefix

  # Threat repo root as a top-level directory or not
  if [[ $SPACESHIP_DIR_TRUNC_REPO == true ]] && spaceship::is_git; then
    local git_root=${$(git rev-parse --absolute-git-dir):h}


This change has been reverted on the current master. Because this requires at least git v2.13.0. Because --absolute-git-dir was added with that release.

slhck · 2018-05-04T07:40:04Z

I see, thanks, I just came here because of #343 (for which there is a workaround). If the main reason for holding off the PR is to avoid possible breaking changes, I would perhaps look into pulling #384 since that seems more innocuous.

My two cents about the default behavior: I like … and I don't think it should be enabled by default.

When the cwd is truncated by SPACESHIP_DIR_TRUNC instead of showing: a/b/c It will show: .../a/b/c The prefix is configurable of course and can be disabled by setting it to an empty string. I also modified SPACESHIP_DIR_TRUNC_REPO to use SPACESHIP_DIR_TRUNC_PREFIX and made it not truncate on a repository that is below the root directory. In addition, I optimized its code to use Zsh constructs instead of command substitutions.

…tion

segevfiner · 2018-05-04T09:41:31Z

Rebased.

salmanulfarzy

Manually tested with Zsh 5.4.2 ang git 2.17.0

	Status	Image
Normal directory	✔️
Git repository	✔️
Subdirectoires inside Git repository	✔️
`.git` directory	❌
Directory with space in name	✔️
Git repository with spaces in name	✔️
Symlinked directories	✔️

I'm approving this PR with the above results. Let's see whether there is any regression on other version of Zsh and git.

segevfiner · 2018-07-11T17:42:36Z

@salmanulfarzy #417 should fix the ".git directory" case.

…paceship-prompt#281)

)

denysdovhan added frozen Progress is temporarily frozen until resolving the specific issue. new-feature A PR that implement feature (section, specific behavior, etc). proposal An issue (rarely PR) for feature-requests, ideas, etc labels Dec 18, 2017

denysdovhan assigned segevfiner Dec 18, 2017

denysdovhan mentioned this pull request Dec 18, 2017

Add SPACESHIP_DIR_TRUNC_PREFIX, a prefix before cwd when truncated #280

Closed

maximbaz reviewed Jan 14, 2018

View reviewed changes

segevfiner force-pushed the dir-trunc-prefix-3.0 branch from 5ec9250 to 4b6bd19 Compare January 15, 2018 20:06

maximbaz mentioned this pull request Jan 19, 2018

Show tags and remotes in git_branch section #293

Merged

segevfiner force-pushed the dir-trunc-prefix-3.0 branch from 4b6bd19 to cdae11d Compare January 19, 2018 13:15

segevfiner changed the base branch from 3.0 to master January 19, 2018 13:15

maximbaz previously approved these changes Jan 19, 2018

View reviewed changes

salmanulfarzy removed the frozen Progress is temporarily frozen until resolving the specific issue. label Jan 20, 2018

segevfiner changed the title ~~[3.0] Add SPACESHIP_DIR_TRUNC_PREFIX, a prefix before cwd when truncated~~ Add SPACESHIP_DIR_TRUNC_PREFIX, a prefix before cwd when truncated Jan 21, 2018

salmanulfarzy mentioned this pull request Jan 22, 2018

Issues with git module when path contains spaces #343

Closed

segevfiner dismissed maximbaz’s stale review via c2b4400 February 27, 2018 18:39

segevfiner force-pushed the dir-trunc-prefix-3.0 branch from cdae11d to c2b4400 Compare February 27, 2018 18:39

salmanulfarzy requested changes Feb 28, 2018

View reviewed changes

salmanulfarzy previously approved these changes Mar 1, 2018

View reviewed changes

salmanulfarzy requested a review from denysdovhan March 2, 2018 10:21

salmanulfarzy mentioned this pull request Mar 4, 2018

fix bug: If git work-tree directory name has spaces, git section will fail and raise an error. #384

Closed

salmanulfarzy reviewed Mar 4, 2018

View reviewed changes

segevfiner dismissed salmanulfarzy’s stale review via 26ed7fd March 5, 2018 20:17

salmanulfarzy previously approved these changes Mar 6, 2018

View reviewed changes

maximbaz previously approved these changes Mar 8, 2018

View reviewed changes

segevfiner dismissed stale reviews from maximbaz and salmanulfarzy via 62d654c March 9, 2018 17:29

segevfiner force-pushed the dir-trunc-prefix-3.0 branch from 26ed7fd to 62d654c Compare March 9, 2018 17:29

denysdovhan reviewed Mar 9, 2018

View reviewed changes

segevfiner force-pushed the dir-trunc-prefix-3.0 branch from 1cf9638 to cf27766 Compare March 10, 2018 16:16

salmanulfarzy previously approved these changes Mar 22, 2018

View reviewed changes

segevfiner dismissed salmanulfarzy’s stale review via 193e407 March 24, 2018 11:50

segevfiner force-pushed the dir-trunc-prefix-3.0 branch from cf27766 to 193e407 Compare March 24, 2018 11:50

salmanulfarzy previously approved these changes Mar 24, 2018

View reviewed changes

salmanulfarzy reviewed May 3, 2018

View reviewed changes

segevfiner added 5 commits May 4, 2018 12:38

Document sorcercy added when implementing SPACESHIP_DIR_TRUNC_PREFIX

1e5a98d

Handle a Git working directory with symlinks correctly in the dir sec…

483d6b1

…tion

Switch to Unicode ellipsis in SPACESHIP_DIR_TRUNC_PREFIX and document

d7fc307

Set SPACESHIP_DIR_TRUNC_PREFIX to disabled by default

c5456d2

segevfiner dismissed salmanulfarzy’s stale review via c5456d2 May 4, 2018 09:38

segevfiner force-pushed the dir-trunc-prefix-3.0 branch from 193e407 to c5456d2 Compare May 4, 2018 09:38

salmanulfarzy approved these changes May 4, 2018

View reviewed changes

salmanulfarzy mentioned this pull request May 25, 2018

expr: syntax error in a git directory containing spaces #452

Closed

salmanulfarzy merged commit d58d98c into spaceship-prompt:master Jul 10, 2018

segevfiner deleted the dir-trunc-prefix-3.0 branch July 11, 2018 17:39

salmanulfarzy pushed a commit to salmanulfarzy/spaceship-prompt that referenced this pull request Jul 14, 2018

Add SPACESHIP_DIR_TRUNC_PREFIX, a prefix before cwd when truncated (s…

1f0b85b

…paceship-prompt#281)

denysdovhan pushed a commit that referenced this pull request May 27, 2021

Add SPACESHIP_DIR_TRUNC_PREFIX, a prefix before cwd when truncated (#281

4bc82db

)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add SPACESHIP_DIR_TRUNC_PREFIX, a prefix before cwd when truncated #281

Add SPACESHIP_DIR_TRUNC_PREFIX, a prefix before cwd when truncated #281

segevfiner commented Dec 15, 2017 •

edited by salmanulfarzy

Loading

denysdovhan commented Dec 18, 2017

maximbaz Jan 14, 2018

maximbaz left a comment

salmanulfarzy commented Jan 22, 2018 •

edited

Loading

salmanulfarzy left a comment

salmanulfarzy Feb 28, 2018 •

edited

Loading

segevfiner Mar 1, 2018

salmanulfarzy Mar 1, 2018

salmanulfarzy Feb 28, 2018

salmanulfarzy left a comment

salmanulfarzy left a comment

salmanulfarzy Mar 4, 2018

segevfiner Mar 5, 2018

salmanulfarzy left a comment

maximbaz left a comment

segevfiner commented Mar 9, 2018

denysdovhan commented Mar 9, 2018

denysdovhan left a comment

segevfiner commented Mar 9, 2018

maximbaz commented Mar 9, 2018

salmanulfarzy commented Mar 9, 2018 •

edited

Loading

segevfiner commented Mar 10, 2018 •

edited

Loading

salmanulfarzy left a comment

salmanulfarzy left a comment

slhck commented May 3, 2018

salmanulfarzy commented May 3, 2018 •

edited

Loading

salmanulfarzy May 3, 2018

slhck commented May 4, 2018

segevfiner commented May 4, 2018

salmanulfarzy left a comment •

edited

Loading

segevfiner commented Jul 11, 2018

Add SPACESHIP_DIR_TRUNC_PREFIX, a prefix before cwd when truncated #281

Add SPACESHIP_DIR_TRUNC_PREFIX, a prefix before cwd when truncated #281

Conversation

segevfiner commented Dec 15, 2017 • edited by salmanulfarzy Loading

denysdovhan commented Dec 18, 2017

maximbaz Jan 14, 2018

Choose a reason for hiding this comment

maximbaz left a comment

Choose a reason for hiding this comment

salmanulfarzy commented Jan 22, 2018 • edited Loading

salmanulfarzy left a comment

Choose a reason for hiding this comment

salmanulfarzy Feb 28, 2018 • edited Loading

Choose a reason for hiding this comment

segevfiner Mar 1, 2018

Choose a reason for hiding this comment

salmanulfarzy Mar 1, 2018

Choose a reason for hiding this comment

salmanulfarzy Feb 28, 2018

Choose a reason for hiding this comment

salmanulfarzy left a comment

Choose a reason for hiding this comment

salmanulfarzy left a comment

Choose a reason for hiding this comment

salmanulfarzy Mar 4, 2018

Choose a reason for hiding this comment

segevfiner Mar 5, 2018

Choose a reason for hiding this comment

salmanulfarzy left a comment

Choose a reason for hiding this comment

maximbaz left a comment

Choose a reason for hiding this comment

segevfiner commented Mar 9, 2018

denysdovhan commented Mar 9, 2018

denysdovhan left a comment

Choose a reason for hiding this comment

segevfiner commented Mar 9, 2018

maximbaz commented Mar 9, 2018

salmanulfarzy commented Mar 9, 2018 • edited Loading

segevfiner commented Mar 10, 2018 • edited Loading

salmanulfarzy left a comment

Choose a reason for hiding this comment

salmanulfarzy left a comment

Choose a reason for hiding this comment

slhck commented May 3, 2018

salmanulfarzy commented May 3, 2018 • edited Loading

salmanulfarzy May 3, 2018

Choose a reason for hiding this comment

slhck commented May 4, 2018

segevfiner commented May 4, 2018

salmanulfarzy left a comment • edited Loading

Choose a reason for hiding this comment

segevfiner commented Jul 11, 2018

segevfiner commented Dec 15, 2017 •

edited by salmanulfarzy

Loading

salmanulfarzy commented Jan 22, 2018 •

edited

Loading

salmanulfarzy Feb 28, 2018 •

edited

Loading

salmanulfarzy commented Mar 9, 2018 •

edited

Loading

segevfiner commented Mar 10, 2018 •

edited

Loading

salmanulfarzy commented May 3, 2018 •

edited

Loading

salmanulfarzy left a comment •

edited

Loading