-
Notifications
You must be signed in to change notification settings - Fork 6k
Improve readability of files in the git changes panel #41857
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
base: main
Are you sure you want to change the base?
Improve readability of files in the git changes panel #41857
Conversation
|
We require contributors to sign our Contributor License Agreement, and we don't have @alphathekiwi on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'. |
|
@cla-bot check |
|
The cla-bot has been summoned, and re-checked this pull request! |
|
Thanks for opening a PR! We're discussing internally whether this design is something we want for the git panel, and will let you know what the outcome of that discussion is. |
|
Hi @alphathekiwi , I like your idea. Previously, I saw there is a similar PR #27230 before. It was being closed due to Zed team would like to handle this case with a different design for Tree-View Git Panel in this ticket #35803. I have been working on that and get a initial setup, but I think the same UI issue is still exist since there is a switch button on top so that you can click on it switch between the tree-view or flat-view as shown here
If you switch to Flat-View you will get the same design as right now, which means the readability of files in git changes are still not great!. |
I didn't know about #27230 so the fact that two people wanted this code change enough to make it into a PR and each with different reasons probably indicates there is a non zero amount of people who would like this to exist in some form or fashion. This is my first attempt to make a PR and I don't really understand how the Zed settings system works, otherwise I probably would have made this a setting. Regardless of the outcome though I want to take the time to thank you for your comprehensive reply and for also taking the time to be very welcoming, I hope to contribute more to the project in future. |
|
I think this makes sense, but as a setting (there are some projects where the directory contains a bunch of index.js and tests.js and you need to see the project folder to know). I do think this makes sense as a default (it'd be better for most projects I use) even though it breaks the obvious sorting behaviour. I know there's also work going on for the tree view, and other display options here so maybe something like: But defer to @cole-miller who's working on this |
|
@cole-miller at your convenience I would love to pair on making this have a setting, I loosely understand how settings work currently with the exception of the Settings UI portion and would love to work on this, I am hanging out in the zed/git1.0 channel fairly regularly most mornings NZT (afternoon/evenings in American time) |
|
@alphathekiwi Sorry for the delay! We'd like to help get this ready to merge, could you book a pairing time here? https://cal.com/esther-trapadoux-zed/30min |
|
@alphathekiwi To add a setting to the settings ui you mainly have to add it to this Vec zed/crates/settings_ui/src/page_data.rs Lines 22 to 23 in 5dde87d
|
86b828b to
10a4c41
Compare
Cool have added settings now have also allowed an option for: |
10a4c41 to
c71ea03
Compare
| SettingsPageItem::SettingItem(SettingItem { | ||
| title: "Show Change Counters", | ||
| description: "When to show change counters in the git gutter.", | ||
| field: Box::new(SettingField { | ||
| json_path: Some("git.show_change_counters"), | ||
| pick: |settings_content| settings_content.git.as_ref()?.show_change_counters.as_ref(), | ||
| write: |settings_content, value| { | ||
| settings_content.git.get_or_insert_default().show_change_counters = value; | ||
| }, | ||
| }), | ||
| metadata: None, | ||
| files: USER, | ||
| }), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like a different setting than the GitPathStyle setting your PR added. Is this a setting that was missing from the settings UI?
The second to last step to finalizing this PR is adding the GitPathStyle setting to the UI
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like CICD is failing because show_change_counter isn't a real value. I'll be happy to help you fix this though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahhh sorry about that, its a combination of broken rust-analyser and trying to work on two things at once.
crates/settings_ui/README.md
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having a file like this would be very helpful, but it should be added through its own PR.
I also think this file is a bit too information-dense in some areas and misses the why the settings are structured the way they are. I'll be happy to help you clean it up for a different PR 😀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay sounds good :)
crates/git_ui/src/git_panel.rs
Outdated
| h_flex() | ||
| .items_center() | ||
| .flex_1() | ||
| // .overflow_hidden() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While we're editing this code could you also please remove this comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
af64793 to
c166537
Compare
c166537 to
a3041c2
Compare
a3041c2 to
3193aa8
Compare



Closes unknown
This PR places the file_name before the file_path so that when the panel is slim it is still usable, mirrors the behaviour of the file picker (cmd+P)
Release Notes: