romkatv / powerlevel10k

A Zsh theme
MIT License
46.59k stars 2.19k forks source link

git status behavior from starship #2526

Closed amogus07 closed 10 months ago

amogus07 commented 10 months ago

I've been using starship, but since I only use zsh and p10k is specifically designed for it, I decided to give it a try. However, there's one thing I miss from starship, that being separate indicators for added, renamed and deleted files. This is my configuration for the git_status module of starship:

[git_status]
format = "[[$conflicted](yellow)[$untracked](218)[$modified$renamed](255)[$staged](green)[$deleted](red)($ahead_behind$stashed)]($style)"
conflicted = " $count "
untracked = " $count "
modified = " $count "
deleted = " $count "
staged = " $count "
renamed = " $count "
stashed = " $count "
style = "cyan"

This is an example of starship's behaviour:

  example 
❯ :> example
  example  1 
❯ ga example               
  ╰─> forgit::add example
A  example
  example  1 
❯ git commit -m "add example"                   
[master (root-commit) 6556e59] add example
 1 file changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 example
  example 
❯ git mv example renamed-example
  example  1 
❯ rm renamed-example 
  example  1  1 
❯

This is what the same sequence looks like in p10k with some customization:

  ~/src/example    master 
❯ :> example
  ~/src/example    master  1 
❯ ga example
  ╰─> forgit::add example
A  example
  ~/src/example    master  1 
❯ git commit -m "add example"
[master (root-commit) 1931858] add example
 1 file changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 example
  ~/src/example    master 
❯ git mv example renamed-example
  ~/src/example    master  2 
❯ rm renamed-example
  ~/src/example    master  2  1 
❯ 

As you can see, renamed and deleted files aren't recognized properly and instead show up as modified. Is it possible to get p10k to behave like starship in this example? Or am I missing something?

romkatv commented 10 months ago

Powerlevel10k uses gitstatus to retrieve the state of git repos. You can see what gitstatus reports by running the following command:

typeset -pm 'VCS_*' | LC_ALL=C sort

If you run it, you'll notice VCS_STATUS_NUM_UNSTAGED_DELETED in the output. Now, you can open ~/.p10k.zsh, find my_git_formatter and edit it to incorporate VCS_STATUS_NUM_UNSTAGED_DELETED where you like.

There is a bit of more info here: https://github.com/romkatv/gitstatus/issues/82

amogus07 commented 10 months ago

Thanks for the quick response! This is my current setup, incorporating your suggestions:

    if (( $1 )); then
      # Styling for up-to-date Git status.
      local       meta='%f'     # default foreground
      local      clean='%76F'   # green foreground
      local      added=$clean   # same as clean
      local    deleted='%196F'  # red foreground
      local   modified='%178F'  # yellow foreground
      local   unstaged='%255F'  # white foreground
      local  untracked='%39F'   # blue foreground
      local conflicted='%208F'  # orange foreground
    [...]
    #  42 if have stashes.
    (( VCS_STATUS_STASHES        )) && res+=" ${clean} ${VCS_STATUS_STASHES}"
    # 'merge' if the repo is in an unusual state.
    [[ -n $VCS_STATUS_ACTION     ]] && res+=" ${conflicted}${VCS_STATUS_ACTION}"
    #  42 if have merge conflicts.
    (( VCS_STATUS_NUM_CONFLICTED )) && res+=" ${conflicted} ${VCS_STATUS_NUM_CONFLICTED}"
    #  42 if have staged changes.
    (( VCS_STATUS_NUM_STAGED - VCS_STATUS_NUM_STAGED_NEW - VCS_STATUS_NUM_STAGED_DELETED )) && res+=" ${modified} $(( VCS_STATUS_NUM_STAGED - VCS_STATUS_NUM_STAGED_NEW - VCS_STATUS_NUM_STAGED_DELETED ))"
    #  42 if have staged additions.
    (( VCS_STATUS_NUM_STAGED_NEW )) && res+=" ${added} ${VCS_STATUS_NUM_STAGED_NEW}"
    #  42 if have staged deletions.
    (( VCS_STATUS_NUM_STAGED_DELETED )) && res+=" ${deleted} ${VCS_STATUS_NUM_STAGED_DELETED}"
    #  42 if have unstaged changes.
    (( VCS_STATUS_NUM_UNSTAGED - VCS_STATUS_NUM_UNSTAGED_DELETED )) && res+=" ${unstaged} $(( VCS_STATUS_NUM_UNSTAGED - VCS_STATUS_NUM_UNSTAGED_DELETED ))"
    #  42 if have unstaged deletions.
    (( VCS_STATUS_NUM_UNSTAGED_DELETED )) && res+=" ${unstaged} ${VCS_STATUS_NUM_UNSTAGED_DELETED}"

So, still no plans of adding a variable for staged files renamed with git mv?

romkatv commented 10 months ago

So, still no plans of adding a variable for staged files renamed with git mv?

Correct.

amogus07 commented 10 months ago

@romkatv How about VCS_STATUS_NUM_STAGED_MODIFIED , VCS_STATUS_NUM_UNSTAGED_ADDED and VCS_NUM_UNSTAGED_MODIFIED? The first one at least should be simple...

romkatv commented 10 months ago

If you send a clean PR, I'll merge it.

amogus07 commented 10 months ago

@romkatv which repo? Here or gitstatus?

romkatv commented 10 months ago

gitstatus if you modify its source code, and powerlevel10k if you modify that one.

amogus07 commented 10 months ago

I think I made all necessary changes to add support for VCS_NUM_STAGED_MODIFIED here. Now, before I send a PR, how can I test it to make sure things work?

romkatv commented 10 months ago

Your code looks good to me. The new field in the output of gitstatusd should be the last one for backward compatibility but other than that I don't see any issues.

To test your changes you first need to build gitstatusd: ./build -w. Then run ./usrbin/gitstatusd --help and see the example at the bottom. You'll also need to update this example in the source code if you are adding a new field.

By the way, are you sure you need VCS_NUM_STAGED_MODIFIED? Are there important use cases for you where VCS_STATUS_NUM_STAGED - VCS_STATUS_NUM_STAGED_NEW - VCS_STATUS_NUM_STAGED_DELETED won't suffice? Can you provide an example?

P.S.

You might find something useful in https://github.com/romkatv/gitstatus/issues/64.

amogus07 commented 10 months ago

Your code looks good to me. The new field in the output of gitstatusd should be the last one for backward compatibility but other than that I don't see any issues.

Yeah, I kinda suspected moving stuff around could break things, ig that's the drawback of using a positional array instead of an associative one… To test your changes you first need to build gitstatusd: ./build -w. Then run ./usrbin/gitstatusd --help and see the example at the bottom. You'll also need to update this example in the source code if you are adding a new field.

I tried that, but it didn't work. The linker doesn't seem to find the right libraries, even though they're installed (my host OS is Fedora):

/usr/bin/ld: cannot find -lstdc++: No such file or directory
/usr/bin/ld: cannot find -lm: No such file or directory
/usr/bin/ld: cannot find -lc: No such file or directory
collect2: error: ld returned 1 exit status

Then I tried running ./build -swd podman, but it gave me another error:

/bin/sh: .: line 240: can't open '/out/build.info': Permission denied

By the way, are you sure you need VCS_NUM_STAGED_MODIFIED? Are there important use cases for you where VCS_STATUS_NUM_STAGED - VCS_STATUS_NUM_STAGED_NEW - VCS_STATUS_NUM_STAGED_DELETED won't suffice? Can you provide an example?

True, it's not strictly necessary. But it would just be nice to have there for consistency, so you don't have to fall back on the shell's arithmetic, and I don't see any drawbacks in it either. VCS_STATUS_NUM_UNSTAGED_ADDED would probably be more important, though, since it provides information that can't efficiently be calculated otherwise. Having all 6 variables would allow basically allow choosing between a more summarized and a more detailed view of (un)staged changes without any slightly annoying workarounds within the prompt config. P.S.

You might find something useful in romkatv/gitstatus#64.

Thanks for pointing it out! Although, I had already found that issue and made my changes by basically following along this commit

romkatv commented 10 months ago

True, it's not strictly necessary. But it would just be nice to have there for consistency, so you don't have to fall back on the shell's arithmetic, and I don't see any drawbacks in that either.

The consistency argument isn't strong enough to justify code changes in gitstatusd and a release. I wouldn't call shell arithmetic a fallback: if VCS_STATUS_NUM_STAGED - VCS_STATUS_NUM_STAGED_NEW - VCS_STATUS_NUM_STAGED_DELETED is what you need, then using it is the right thing to do. You might want, however, to simplify your zsh code a bit:

integer num_staged_modified='VCS_STATUS_NUM_STAGED - VCS_STATUS_NUM_STAGED_NEW - VCS_STATUS_NUM_STAGED_DELETED'
(( num_staged_modified )) && res+=" ${modified} $num_staged_modified"

I would also suggest removing non-ASCII characters from your code, especially those from UNICODE Private Use Area.

(( staged_modified )) && res+=" ${modified}"$'\UF459'" $staged_modified"

Now the code can be read and edited anywhere.

romkatv commented 10 months ago

I tried that, but it didn't work.

I'm afraid you would need to improvise.

amogus07 commented 10 months ago

The consistency argument isn't strong enough to justify code changes in gitstatusd and a release.

What if I add the other two variables? Would you accept it then? Or should I just add [...]UNSTAGED_ADDED and leave out the [...]MODIFIED ones?

romkatv commented 10 months ago

I'll merge any PR that satisfies these two requirements:

  1. The code is correct and clean.
  2. There is a clear need for the change. It enables a use case that was previously impossible, difficult, or inefficient.

In https://github.com/romkatv/gitstatus/issues/64#issuecomment-538787855 I wrote this about exposing all counters:

It's hard though, because of renames and blob<->tree changes.

Unfortunately, I don't remember the details. If you add a new counter, please verify that it exactly matches a category from the output of git status (see https://github.com/romkatv/gitstatus/issues/64#issuecomment-538749355). There is currently one exception to this rule: renames are reported as deleted + new. There should not be any new exceptions. In particular, type change and blob<->tree changes must be reported the same way as in git status. It's possible that I've already added all counters that are easy to add under these constraints but maybe not. I wish I wrote more notes when I worked on this.

amogus07 commented 10 months ago

Uh, managed to compile my current code, tested it with a repo I'm working on but it now it seems resp[11] and resp[22] are both 1 even though they should be 3… git status in that repo:

On branch master
Your branch is up to date with 'origin/master'.

Changes to be committed:
  (use "git restore --staged <file>..." to unstage)
    modified:   functions/_lf_help
    modified:   functions/_lf_verbose
    modified:   functions/_mf_define
romkatv commented 10 months ago

Thanks for working on the feature. Just a heads-up, I can only merge pull requests that are ready to go -- complete, correct, and clean. I get that coding can be tricky, but I'm really stretched thin and can't offer detailed help or troubleshooting. If you manage to tackle these issues and the code meets the project's standards, I'm definitely up for reviewing and possibly merging it.

amogus07 commented 10 months ago

@romkatv How about VCS_STATUS_NUM_STAGED_MODIFIED , VCS_STATUS_NUM_UNSTAGED_ADDED and VCS_NUM_UNSTAGED_MODIFIED? The first one at least should be simple...

OOOOH, I just realized that… unstaged new = untracked. So… all of this except (hypothetical) VCS_STATUS_NUM_STAGED_MOVED and VCS_STATUS_NUM_STAGED_COPIED would be basically redundant🤦🏻‍♂️

romkatv commented 10 months ago

That's a good observation. So you have everything to implement a prompt you are after? No changes to gitstatus required?

amogus07 commented 10 months ago

Kinda. It would still be nice to have support for counting moved and copied files though… I did a bit of research on it, and it seems this would be the relevant documentation to start implementing it:

GIT_DELTA_RENAMED and GIT_DELTA_COPIED will only show up if you run git_diff_find_similar() on the diff object.

That function is documented here. I probably wouldn't really be able to do that on my own any time soon, though, since my experience with C++ is basically limited to some Arduino projects (and school starts tomorrow ☠️). But what are some things that would need to be done? (I'll try my best to understand)

romkatv commented 10 months ago

It would still be nice to have support for counting moved and copied files though…

gitstatusd shards repos and treats individual shards as independent. To detect moves gitstatusd would need an extra stage that runs after all shards are processed. What makes it more challenging is the early return support based on the number of detected changes. gitstatusd would need to work with ranges instead of numbers for tracking the number of changes internally during the shard processing phase.

I very strongly discourage you from attempting to implement this.

amogus07 commented 10 months ago

Thanks for the explanation! I think I get what you mean, and Ig I'll live with it for now, at least until I have the time, motivation and experience to take on the challenge (if that ever happens...)