sodapopcan / vim-twiggy

Git branch management for Vim
280 stars 15 forks source link

Checkout non local branch names produces errors #29

Open eglimi opened 5 years ago

eglimi commented 5 years ago

Hi!

Twiggy returns an error when anything than a local branch name is checked out. Thus, there seems to be a problem when checking out one of the following things, even if they point to a local branch.

E.g. in the following case, twiggy returns an error.

% git branch
* (HEAD detached at f7f0335)
  features/binary_id
  master

The error returned is the following.

Error detected while processing function twiggy#Branch[14]..<SNR>86_Render[62]..<SNR>86_standard_view[10]..twiggy#get_branches:
line    6:
E684: list index out of range: 0
E116: Invalid arguments for function add
Error detected while processing function twiggy#Branch[14]..<SNR>86_Render:
line   62:
E712: Argument of extend() must be a List or Dictionary
Already at newest change

Again, let me know if you need more information.

sodapopcan commented 5 years ago

Thank you for your diligent error reporting!

I'm assuming this is a version thing again. I made a change around that stuff when switching commands for getting branches and it's still on the hacky side while I re-organize.

Do either of these commands work for you:

$ git rev-parse --symbolic-full-name --abbrev-ref HEAD
$ git show-ref --hash=9 HEAD
eglimi commented 5 years ago

In this case, I don't think it's a problem with the Git version. Both commands work, but the second one doesn't return anything in my case.

This happens when there is no HEAD ref (which is fine, as far as I know). From the example repo, there are the following refs (I shortened the SHA numbers from the output).

git show-ref
61f5ca refs/heads/features/binary_id
63a27f refs/heads/master
63a27f refs/remotes/origin/master

I noticed that twiggy works fine in repos where there is a HEAD ref. But then, it probably doesn't always show what is expected. For example, in another repository, when I check-out 7eb6263 and open twiggy, it shows HEAD@bc4c15252 as the current local branch. This is also what the above command (show-ref HEAD) returns. In this case, it is not even a local branch, but one from the origin remote.

Would git rev-parse not work better here? For example, if I change the git show-ref command in the plugin to the following, it seems to work fine.

git rev-parse --revs-only --short HEAD
eglimi commented 5 years ago

Just as an idea, since twiggy requires vim-fugitive, there is also FugitiveHead(...). This might be a possible alternative to using a Git command directly.

sodapopcan commented 5 years ago

Ah yes, I should absolutely be using FugitiveHead() and, generally, delegating to fugitive whenever possible. Honestly, I've pushed through these issues pretty quickly as this is the most action twiggy has had recently and haven't had a lot of free time. I'll be able to fix this today, though, or feel free to send a PR if you like.

Thanks again!

sodapopcan commented 5 years ago

I've used your git rev-parse --revs-only --short HEAD for now as FugitiveHead() returns empty string if there are is no HEAD branch. As mentioned, I need to clean up this area of the code and build a better mental model of what the proper thing to do is, so I'm going to leave this open for now.

eglimi commented 5 years ago

Thank you for the fast update again! It works great now :)

Just in case you want to use FugitiveHead() in the future, it accepts an optional parameter for the case of a detached HEAD. On the other hand, the current rev-parse command should be robust and avoids the fugitive dependencies.

sodapopcan commented 5 years ago

Ohhhh, yes, oops 😬, thanks for letting me know! I’d definitely rather use fugitive as I have no plans to ever drop the dependency.