Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Git history and diff view improvements #7128

Merged
merged 4 commits into from
Mar 27, 2024
Merged

Git history and diff view improvements #7128

merged 4 commits into from
Mar 27, 2024

Conversation

mbien
Copy link
Member

@mbien mbien commented Mar 4, 2024 •
edited
Loading

Lets try to make the git history UI a bit more useful.

This PR is mostly about UI/UX but fixes an old branch filtering issue too (#7128 (comment))

screenshots:
git-history-sbs

diff view:
switch-layout

  • vertical / horizontal layout switcher
  • removed loading screen to make the diff load faster :)
  • when switching between diffs (eg using the arrows) the UI state is copied to the new diff (text/graphical mode, divider value etc)
  • git annotation colors for files
  • commit icon instead of folder icon

summary view:
clearer-links

merge-commits-desaturated

  • merge commits are a little bit desaturated and not rendered in bold

quick search filter:
history-all-filter

  • implemented all-filter and made it the default
  • highlighting is now consistently working in the relevant columns
  • even in tooltips
  • some other bug fixes and things I probably forgot to write down

added file filter and path highlighting:
git-history-file-filter

toolbar tooltip of next/prev buttons shows keybindings:
git-history-toolbar-tooltip

@mbien mbien added Need Squashing UI User Interface git [ci] enable versioning job ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) labels Mar 4, 2024
@mbien mbien added this to the NB22 milestone Mar 4, 2024
@mbien mbien mentioned this pull request Mar 4, 2024
Closed
@mbien mbien marked this pull request as ready for review March 4, 2024 03:38
@mbien
Copy link
Member Author

mbien commented Mar 5, 2024 •
edited
Loading

this is still a little bit WIP. I want to check if I can implement a file filter with highlighting etc, and also improve how the table behaves in single-file-history mode (#5680 (comment))

done, added a screenshot of the file filter.

@mbien mbien force-pushed the git-diff-history-improvements branch from 7cf9a42 to aff4bbe Compare March 10, 2024 05:36
@mbien
Copy link
Member Author

mbien commented Mar 10, 2024

I found a few more issues but thats as much I want to do via a single PR.

File filter is now implemented, this required some optimizations - which is a good thing since this made everything faster.

  • the cell renderer code which calculated the longest path of the displayed file paths had O(n²) complexity, this is now linear
  • filter results are cached per revision and are invalidated when the search changes
  • some other smaller optimizations

@mbien
Copy link
Member Author

mbien commented Mar 12, 2024 •
edited
Loading

I added another commit which removes the branch filter of the toolbar. The reason for that is essentially #6594 which never got fully resolved.

The branch filter in the toolbar filtered the results after the git log command ran. This requires to have all branch information in the revisions, if some branches are missing, it is likely that the filter will remove too many commits (it often would show no results at all in larger repos). Removing the branch filter fixes it.

Secondly: there is a second branch filter in the search options section which runs during git log and is therefore not affected. Having two branch filters in the same UI is also a bit questionable, it confused me quite often tbh even when it worked. The typical usecase is to view the history of the checked out branch which is the default anyway.

toolbar branch filter removed with search section expanded:
image

This was referenced Mar 19, 2024
Merged
Merged
@mbien mbien force-pushed the git-diff-history-improvements branch 2 times, most recently from 4fb8227 to ec86036 Compare March 19, 2024 11:28
matthiasblaesing
matthiasblaesing reviewed Mar 19, 2024
View reviewed changes
Copy link
Contributor

@matthiasblaesing matthiasblaesing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the removal of the secondary branch selector is a helpful simplification and in general this is an improvement. I left a few inline comments.

ide/git/src/org/netbeans/modules/git/ui/history/RevisionNode.java Outdated Show resolved Hide resolved
ide/git/src/org/netbeans/modules/git/ui/history/RevisionNode.java Outdated Show resolved Hide resolved
ide/versioning.util/src/org/netbeans/modules/versioning/history/SummaryCellRenderer.java Show resolved Hide resolved
@mbien
Copy link
Member Author

mbien commented Mar 23, 2024 •
edited
Loading

things I haven't implemented:

  • I wanted to add file type icons to the diff view but couldn't find an elegant way of doing this
  • diff view file name column has no quicksearch highlighting due to the way how the tree works, I left a comment in-code
  • I haven't touched the file history window at all otherwise this PR would be too large. It doesn't listen to hotkeys etc. (NB has too many diff windows!)
  • UI persistence. Layout and table column settings are not persisted.

@mbien
Copy link
Member Author

mbien commented Mar 26, 2024

going to squash/rebase to fewer commits and make this ready to merge soon

@mbien
git history diff-view improvements
366915c 
 - copy tab and divider state when diffs are switched
 - remove loading screen, simply swap out the diff views
   - git diff generation is very fast
   - this avoids flickering when switching between diffs
 - added vertical / horizontal layout switcher
 - commit tree nodes use now the commit icon
 - files are colored based on their git add/remove/modify state
 - add key binding to next/prev button tooltip text
@mbien
git history summary-view improvements
7e27a8e 
add collapse commit message link

 - message could only be expanded in past
 - make the expand link more obvious

improve path length computation performance

 - path length state is stored in RevisionItemCell so that it doesn't
   have to be constantly recomputed in EventRenderer
 - removal of the 20 path limit fixes the layout of larger commits
 - make the event link a bit more obvious
 - desaturate merge commit entries as originally intended
@mbien
improve the quick search and search highlighting
229bf2a 
 - implemented 'All' and 'File' filter and made 'All' the default
 - fixed search highlighting for the diff view's msg and author columns
 - fixed search highlighting in selected summary entries
 - implemented search highlighting for paths
 - highlighting works now in tooltips too

performance: add file filter and results cache

 - added search result cache for significantly improved performance
 - try to avoid redundant computations and other tweaks
@mbien
remove branch filter from toolbar
d5d6ab8 
 - branch filter already exists in the search options section
 - having two branch filters can be confusing
 - jgit allows currently to only mark 24 revs during rev walk. This
   means that the branch filter could only work with branch
   information for 23 branches max. This would cause it to incorrectly
   filter out commits in repos with many branches and can lead to no or
   wrong results. The branch filter of the search options section is
   applied during the git command and doesn't have this issue.
@mbien mbien force-pushed the git-diff-history-improvements branch from a1c63a0 to d5d6ab8 Compare March 26, 2024 21:57
@mbien mbien removed Need Squashing ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) labels Mar 26, 2024
@mbien mbien merged commit e074d9d into apache:master Mar 27, 2024
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lkishalmi lkishalmi lkishalmi left review comments

@matthiasblaesing matthiasblaesing matthiasblaesing left review comments

Assignees
No one assigned
Labels
git [ci] enable versioning job UI User Interface
Projects
None yet
Milestone
NB22
Development

Successfully merging this pull request may close these issues.

GIT: Large empty space between the file path and the Show Actions button
3 participants
@mbien @lkishalmi @matthiasblaesing