Powerlevel9k / powerlevel9k

Powerlevel9k was a tool for building a beautiful and highly functional CLI, customized for you. P9k had a substantial impact on CLI UX, and its legacy is now continued by P10k.
https://github.com/romkatv/powerlevel10k
MIT License
13.47k stars 947 forks source link

vcs segment shows incorrect remote branch when tracking remote isn't called "origin" #1209

Open romkatv opened 5 years ago

romkatv commented 5 years ago

Tested on master with the default config.

  1. Clone a repo from github.
  2. cd into it.
  3. git remote rename origin source

Expected: There is no remote branch in the vcs prompt (because remote is the same as local). Actual: vcs prompt shows remote branch "source/master".

dritter commented 5 years ago

Hey @romkatv .

I've been thinking about this, since I implemented the remote name in gitstatus and used that to prepend the branch name if the remote name differs from origin. And that leads to the same error in the gitstatus segment. And, to be honest, I don't know how if we can fix this. I don't see a proper way to see if a remote is "owned" by the user or not. As far as I know, git makes not difference between a renamed remote or a remote that was just added. All remotes are equal (I might be wrong). But if this is the case, the only thing we could do about that is to check for origin and show the remote name otherwise.

Syphdias commented 5 years ago

Maybe I'm reading this wrong, but what exactly is wrong? image

romkatv commented 5 years ago

Maybe I'm reading this wrong, but what exactly is wrong?

Oops, sorry about that. I should've tried my own reproduction instructions before posting them. This should demonstrate the issue:

git remote rename origin my/source

Full setup:

docker run -e LANG=C.UTF-8 -e LC_ALL=C.UTF-8 -e TERM=$TERM -it --rm ubuntu bash -uexc '
  apt update && apt install -y zsh git
  git clone https://github.com/bhilburn/powerlevel9k.git ~/powerlevel9k
  cd ~/powerlevel9k
  git remote rename origin my/source
  echo "
    POWERLEVEL9K_LEFT_PROMPT_ELEMENTS=(vcs)
    POWERLEVEL9K_RIGHT_PROMPT_ELEMENTS=()
    source ~/powerlevel9k/powerlevel9k.zsh-theme" >~/.zshrc
  zsh -i'

wrong-remote

Note that branches also can have slashes in their names, so it's not possible to extract branch name from the symbolic name of a remote reference. That is, if git rev-parse --verify HEAD@{upstream} --symbolic-full-name gives you foo/bar/baz/qux, branch name can be any of qux, baz/qux or bar/baz/qux.

romkatv commented 5 years ago

And, to be honest, I don't know how if we can fix this. I don't see a proper way to see if a remote is "owned" by the user or not.

Oh, now I see why you wrote that code with special-casing "origin". You want to distinguish between remotes owned by you and remotes owned by others?

Names of the remotes have no meaning on their own. A user can assign meaning to them but it's not enforced by git and thus won't be shared by other users. A theme shouldn't assume predefined meaning, at least not with default settings, so any mention of "origin" in the code is likely at error.

To distinguish between your and not-your repositories you could look at the remote URL. If the repo is under https://github.com/dritter/ or git@github.com:dritter/, it's yours, otherwise it isn't. This requires a configuration option to specify the ownership predicate.

Syphdias commented 5 years ago

Names of the remotes have no meaning on their own. A user can assign meaning to them but it's not enforced by git and thus won't be shared by other users. A theme shouldn't assume predefined meaning, at least not with default settings, so any mention of "origin" in the code is likely at error.

Agreed, and it does not show up in code. (only in (old) comments)

And, to be honest, I don't know how if we can fix this. I don't see a proper way to see if a remote is "owned" by the user or not.

Oh, now I see why you wrote that code with special-casing "origin". You want to distinguish between remotes owned by you and remotes owned by others?

I don't think that's the idea. The idea is to show the remote branch if the name differs. So what it should check is, if the tracking branch is refs/remotes/$remote/$localbranchname. If that doesn't exists, it should display the remote branch as $remote/$remotebranchname.

This should(tm) not collide with the issue of not not knowing where the remote ends and where the branch names start since this combination is unique locally, maybe??? (No deep research done just checked my .git/{refs/remotes/,config,FETCH_HEAD})

I don't know how to check this though. Is there a way to get just the remote without the remote branch? git rev-parse --abbrev-ref HEAD@{upstream} and git rev-parse HEAD@{upstream} --symbolic-full-name always display the branch as well. It's not enough to check if the the output ends with /$localbranchname. As you said: bar/baz/qux as remotebranch and qux as localbranch could mean the remote is bar with a renames local branch (show remote branch). Or the remote could be bar/baz which would mean the localbranch was not renamed (don't show remote branch).

romkatv commented 5 years ago

Names of the remotes have no meaning on their own. A user can assign meaning to them but it's not enforced by git and thus won't be shared by other users. A theme shouldn't assume predefined meaning, at least not with default settings, so any mention of "origin" in the code is likely at error.

Agreed, and it does not show up in code. (only in (old) comments)

And, to be honest, I don't know how if we can fix this. I don't see a proper way to see if a remote is "owned" by the user or not.

Oh, now I see why you wrote that code with special-casing "origin". You want to distinguish between remotes owned by you and remotes owned by others?

I don't think that's the idea. The idea is to show the remote branch if the name differs. So what it should check is, if the tracking branch is refs/remotes/$remote/$localbranchname. If that doesn't exists, it should display the remote branch as $remote/$remotebranchname.

Hm... if @dritter wanted this, he would've written it so:

if [[ -n $VCS_STATUS_REMOTE_BRANCH &&
      $VCS_STATUS_REMOTE_BRANCH != VCS_STATUS_LOCAL_BRANCH ]]; then
  local r="$VCS_STATUS_REMOTE_NAME/$VCS_STATUS_REMOTE_BRANCH"
  segment_content+=("${__P9K_ICONS[GITSTATUS_REMOTE_BRANCH]}$r")
fi

But in fact he has written something much more complex. I don't understand what he's trying to achieve with that code but based on the discussion at https://github.com/bhilburn/powerlevel9k/pull/1229#discussion_r269640779 I though he wants to distinguish between his own remotes (which are usually "origin") and other remotes (he used "ben" as an example).

@dritter It would help if you could clarify.

dritter commented 5 years ago

TBH, my first idea was indeed to distinguish between my branches and foreign ones. But as you, @romkatv , correctly pointed out, this is not possible without the help of the user.. My use case is to get a hint of I am working on a branch that, if pushed, would probably break things (in my case: I have write access to this repo, and I could push to next, which is rarely what I want).

A good alternative to that is @Syphdias suggestion. Could either of you create a PR? That would be nice.

Syphdias commented 5 years ago

@dritter So there are two ideas here:

  1. The initial one where you could define a lookup to find "owned" remotes. I'd say it could be a new feature. Maybe discussed in a new Issue?
  2. locally differently named branch Well, the pseudo code from @romkatv looks easy. I think the VCS_* variables are probably output from gitstatus(?) and I didn't find an equivalent for VCS_STATUS_REMOTE_BRANCH with vcs_info but I understand little of its workings. I came up with a simple modification. But I'm not sure if it achieves exaclty what is wanted here. For example it will unfortunatly show no remote if the remote is my/source, upstream/tacking branch is master and the local branch is source/master. I would need a variable with only the remote (without branch name) in it for it to be completely correct... Tell me how and I'll open a PR ;)
diff --git a/segments/vcs/vcs.p9k b/segments/vcs/vcs.p9k
index de678514..b4214a5c 100644
--- a/segments/vcs/vcs.p9k
+++ b/segments/vcs/vcs.p9k
@@ -209,7 +209,7 @@ function +vi-git-remotebranch() {
   if [[ -n ${remote} \
       && ( \
         "${P9K_VCS_GIT_ALWAYS_SHOW_REMOTE_BRANCH}" == 'true' \
-        || "${remote#*/}" != "${branch_name#heads/}" \
+        || ! "${remote}" =~ "/${branch_name#heads/}$" \
       ) ]]; then
     hook_com[branch]+="${__P9K_ICONS[VCS_REMOTE_BRANCH]}${${remote// /}//\%/%%}"
   fi
romkatv commented 5 years ago

Well, the pseudo code from @romkatv looks easy. I think the VCS_* variables are probably output from gitstatus(?) [...]

It was meant to be real code to replace this. I haven't tested it though.

dritter commented 5 years ago

@Syphdias agreed on idea 1. That should be a new feature.

And about your second idea: I wouldn't change the vcs segment.

@romkatv in gitstatus you strip away the remote name. So could we distinguish between the remotes? Coming back to my initial example: if I track a branch next from remote x and name that next locally, is there a difference between VCS_STATUS_REMOTE_BRANCH and VCS_STATUS_LOCAL_BRANCH? I can't check that, currently on my phone..

romkatv commented 5 years ago

@dritter I'm not sure I understand the question.

If HEAD is a branch, then VCS_STATUS_LOCAL_BRANCH is the branch name. It's the named marked with an asterisk in the output of git branch.

git-branch

If the branch is tracking a remote branch, then both VCS_STATUS_REMOTE_NAME and VCS_STATUS_REMOTE_BRANCH are non-empty. VCS_STATUS_REMOTE_NAME is one of the remotes you can see in the output of git remote.

git-remote

$VCS_STATUS_REMOTE_NAME/$VCS_STATUS_REMOTE_BRANCH is what you see in brackets in the output of git branch -vv.

git-branch-vv

Or to put it another way, if HEAD is a branch that tracks a remote branch, then the output of git status -sb can be reproduced with "## $VCS_STATUS_LOCAL_BRANCH..$VCS_STATUS_REMOTE_NAME/$VCS_STATUS_REMOTE_BRANCH".

git-status-sb

Concatenating strings is easier than splitting, so it's fairly easy to do anything you want with the data gitstatus provides. There is also VCS_STATUS_REMOTE_URL, which you could use for ownership checks, if desired.

Syphdias commented 5 years ago

@romkatv I didn't even take a look at the gitstatus segment since you reported it for vcs and I just looked at that.

And about your second idea: I wouldn't change the vcs segment.

@dritter Are you saying there is no bug here or just a "won't fix"? The behaviour original intended is no possible as we agreed on. But what is it, it is supposed to do then?

If the intention is to show differently named local branches, it is very straight forward to fix in gitstatus, just copy paste @romkatv's code since it already provides VCS_STATUS_REMOTE_BRANCH that can easily be compared to VCS_STATUS_LOCAL_BRANCH. For vcs I don't know, since I couldn't find a VCS_STATUS_REMOTE_BRANCH-equivalent in vcs_info.

If this is not wanted (currently buggy) behaviour. What should it do? Display the remote ref if there is a/ in the remote name?

romkatv commented 5 years ago

@romkatv I didn't even take a look at the gitstatus segment since you reported it for vcs and I just looked at that.

Yeah, it's confusing because we have two parallel threads here. One is the original bug I reported against vcs, and another is about differentiating between remotes in gitstatus that follows from @dritter's comment (https://github.com/bhilburn/powerlevel9k/issues/1209#issuecomment-478402344). All your comments are from the first conversion, while all (or almost all) @dritter's are from the second. I'm replying to both, so I'm confusing everyone.

dritter commented 5 years ago

Haha. Sorry for contributing to the confusion here.

I'll try to bring some light into the dark here:

About vcs segment showing the wrong remote

I think this is more a display issue how we want to display the remote name. In @romkatv screenshot the remote name is mangled with the remote branch, and, if I understood correctly, this is the original issue here. I'll take a deeper look tomorrow.

About remote name in gitstatus segment

... VCS_STATUS_REMOTE_BRANCH that can easily be compared to VCS_STATUS_LOCAL_BRANCH.

I disagree with that one. Local and remote branch might be the same (see my use case below).

Use case

Steps in short:

To illustrate my use case, I use this repo (bhilburn/powerlevel9k) and Syphdias fork (syphdias/powerlevel9k) as an example (my own fork is a bad example, because I switched the default branch in github). So I cloned syphdias fork (master branch) and added the original repo (bhilburn/powerlevel9k) as remote. After that I checkout bhilburn/next.

If I now do gitstatus_query P9K, $VCS_STATUS_LOCAL_BRANCH and $VCS_STATUS_REMOTE_BRANCH both show next, which is why I started comparing against origin (here is where I went wrong). And this is the loop hole in @romkatv code snippet.

I hope that clears it a bit up, and does not add more confusion.

Syphdias commented 5 years ago

@dritter, could you describe when and how the right part should be displayed? If there is more than one remote? So for your example: If you're on master: master→origin/master If you're on next: next→bhilburn/next What is not possible imho, is to only show the remote branch for the second thing since we can't know that origin is "your own" (at least not without the detection via url or similar).

Edit: Probably like that if you want to include different local branch names If "always show tracking" is on -> show tracking if one remote + local branch name is different -> show tacking if multiple remotes (+ not your own remote) -> show tracking if detection is possible: multiple remotes + own remote + different local name -> show tracking else -> not tracking

PS: Adding to the confusion: my master is closer to upstream next because of an old PR. 😜 And why is specifying harder than programming...

dritter commented 5 years ago

Having slept one night about this problem, I think my use case is an edge case and is not solvable. My use case is about ownership, which git does not know about, hence we need user input for this case. I'll think some more about this and create a new PR, if I got to somewhere..

romkatv commented 5 years ago

Seems like the level of confusion is getting boringly low here. Let's bump it up a notch.

After that I checkout bhilburn/next.

If I now do gitstatus_query P9K, $VCS_STATUS_LOCAL_BRANCH and $VCS_STATUS_REMOTE_BRANCH both show next...

If you literally run git checkout bhilburn/next, you'll end up in detached HEAD state and not on any branch. VCS_STATUS_LOCAL_BRANCH and VCS_STATUS_REMOTE_BRANCH will be empty. Symbolic reference refs/remotes/bhilburn/next will resolve to the same commit as HEAD, but it doesn't mean you are on bhilburn/next branch. If your local next branch is up to date with bhilburn/next, symbolic reference refs/heads/next will also resolve to the same commit as HEAD, but this also doesn't mean you are on next branch.

gitstatus has straightforward logic here. VCS_STATUS_LOCAL_BRANCH is empty if and only if the repo is in detached HEAD state. VCS_STATUS_REMOTE_BRANCH and VCS_STATUS_REMOTE_NAME are empty if and only if you are not on a branch with a tracking remote.

P9K has different logic. What it shows as local branch name isn't always a branch. For example, if you run git checkout master && git checkout HEAD~0, you'll be in detached HEAD with P9K showing heads/master. After git checkout HEAD~1 it'll either become master~1 (note the inconsistency with the previous display, which wasn't master~0) or heads/some-other-branch, depending on what other branches you have. However, if you end up on a tagged commit, then local branch name will be different in both cases -- it'll be just the commit hash.

This diversity of local branch name formats and meanings makes it challenging for users to build a consistent model for interpreting what they are seeing. If P9K says I'm on heads/master, it could mean I'm on a branch named heads/master and therefore my commits will move HEAD, or it can mean I'm in a detached HEAD state at the same commit as master branch. There is no way to tell.

dritter commented 5 years ago

If you literally run git checkout bhilburn/next, you'll end up in detached HEAD state and not on any branch.

Yeah. I forgot a -t.. If you do a git checkout -t bhilburn/next, you'll end up in the state I described.

This diversity of local branch name formats and meanings makes it challenging for users to build a consistent model for interpreting what they are seeing.

Full Ack. We should try to make that as consistent as possible. I'll have a look into the vcs segment.