Skip to content

Basic git improvements #1082

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Apr 14, 2025
Merged

Basic git improvements #1082

merged 6 commits into from
Apr 14, 2025

Conversation

rukins
Copy link
Contributor

@rukins rukins commented Apr 12, 2025

Thanks to @Euphrasiologist for the prompt!

I found some bugs, and here are a few improvements.

  1. Fix checking if currently in git repo (Adding git branch in script doesn't work #1079)
  2. Use git branch --show-current instead of git rev-parse --abbrev-ref HEAD to get current branch
  3. Refactor processing current status (relying on https://git-scm.com/docs/git-status). Before, in some cases it leads to unexpected behavior. (for example, when renaming or deleting files)

@Euphrasiologist
Copy link
Contributor

Thanks for updating it! Glad someone found it useful :)

@fdncred
Copy link
Contributor

fdncred commented Apr 12, 2025

Looks like more than an update. Quite a bit has changed including less coloring. Is that right?

@rukins
Copy link
Contributor Author

rukins commented Apr 12, 2025

@fdncred, yes, just made it look more like original output of git status

@fdncred
Copy link
Contributor

fdncred commented Apr 12, 2025

I agree that there are some improvements here but I'm not really a fan of changing the coloring. That's the UI part for shells so it's kind of important. Some of these changes seem to be just preferences. It would be nice to include the original authors preferences but allow others to tweak to their own preferences. Otherwise, we lose most of the work the original author put into this.

@rukins
Copy link
Contributor Author

rukins commented Apr 12, 2025

well, what can @Euphrasiologist say about this? Getting the original coloring back is not a problem

@Euphrasiologist
Copy link
Contributor

The colours were what I personally wanted when I made it, and I agree it's nice if they are retained. But I'm happy either way 😊

@rukins
Copy link
Contributor Author

rukins commented Apr 13, 2025

okay, returned the branch color to original, but, as previous version doesn't consider staged and unstaged status, changes for status coloring seem reasonable. please take it into account, @fdncred

@fdncred
Copy link
Contributor

fdncred commented Apr 13, 2025

okay, returned the branch color to original, but, as previous version doesn't consider staged and unstaged status, changes for status coloring seem reasonable. please take it into account, @fdncred

I guess maybe I'm missing something because I don't see the original colors in the PR. yellow_bold and blue_bold aren't anywhere in it anymore.

@rukins
Copy link
Contributor Author

rukins commented Apr 14, 2025

I am not sure it's a good design, but I tried to take the colors back

@fdncred
Copy link
Contributor

fdncred commented Apr 14, 2025

Thanks

@fdncred fdncred merged commit c639113 into nushell:main Apr 14, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants