Feature. Advanced Commit View: Filter files text field. #669

lolgear · 2020-04-12T18:39:11Z

Live repository file pattern variable has been added.
Advanced commit view search text field has been added.

Well, I would like to introduce another missing feature: File filtering. It is especially use in large commits.

I would like to have a feedback about this PR and feedback about roadmap of this feature.

Why it is not a final release?
I would like to add filtering functionality to each view ( view controller ) that requires it. Most of view controllers are consists of DiffFiles and DiffContents view controllers. However, it is not very simple to add this functionality into these small controllers.

So, I would like to add this functionality via inheritance from GIViewController or GIComposedViewController ( new entity ).

Add new entity GIComposedViewController.
Inherit all "final" controllers from this new entity.
Add filtering functionality into base class.
Add flags to enable/disable filtering functionality.
Add key bindings ( like alt + F or nothing + F ).

Fixes:

Further improvements:
We could add this functionality to GIDiffFilesViewController. If we do this, we could add search text field on every view that uses GIDiffFilesViewController. However, we should add also flag to indicate if we need this searchField or not.

lolgear · 2020-04-12T18:40:36Z

@lucasderraugh Lets discuss!

lolgear · 2020-04-13T16:51:49Z

@lucasderraugh What do you think about it?

lucasderraugh · 2020-04-14T05:36:52Z

I’ll check it out on Tuesday.

lucasderraugh · 2020-04-15T00:01:23Z

Very cool! Here's some of my thoughts on it:

I think we should try to mirror the Xcode behavior where the search is in the bottom left of the section it represents not across the entire view.

I think we need some protection from committing while filtering. If you accidentally have filtering on it can look like you're not committing what you think you're committing. Perhaps we should have a warning if we're hiding content that would be added to the commit.

Overall, this is pretty exciting.

lolgear · 2020-04-15T10:58:41Z

@lucasderraugh Could you draw an example of what you want?
Since file pattern is applied to live repository, all controllers on advanced commit view will be affected and will get updates.
So, filtering is applying to all controllers.
Left bottom corner will be confusing for user, no?

Also, we should not forget to reset search on window disappearing, because other controllers should not know about filtering.

lucasderraugh · 2020-04-15T18:32:00Z

Something like this, similar to how Xcode filters the content on the left pane:

I don't think the other filter buttons are necessary though, just the text field.

lolgear · 2020-04-15T19:48:58Z

@lucasderraugh
I embedded search text field after index view, so, now I can't reach message text view on tab. However, if I set focus on message text view and press shift + tab, everything works fine and focus jumps from message to search text field and up to index and finish at working directory.

How could I set nextKeyView / firstResponder for search text field? ( Yes, I set both in setup method ).

lolgear · 2020-04-18T12:13:07Z

@lucasderraugh
Could you help with these two bugs:

I can't set any nextKeyView/firstResponder to make everything works fine, searchTextField has nextKeyView as WorkDirectory no matter what I set.
How to filter textDidChange:(NSNotification *)notification for concrete NSSearchTextField? I see that notification.object is not equal to self.searchTextField and I could check it only for following: [[self.searchTextField subviews].firstObject subviews] == notification.object

lolgear · 2020-04-18T13:05:51Z

@lucasderraugh
Haha, I just change text handling from notification center to delegate and...
Everything works fine now :D

Could you check it?

lucasderraugh · 2020-04-23T21:27:10Z

How to filter textDidChange:(NSNotification *)notification for concrete NSSearchTextField? I see that notification.object is not equal to self.searchTextField and I could check it only for following: [[self.searchTextField subviews].firstObject subviews] == notification.object

Not sure about the first issue, but the second is because the object sending out the change is probably coming from the field editor (NSTextView). You should just query the current string value on searchField when receiving that update most likely.

lolgear · 2020-04-23T21:29:27Z

@lucasderraugh I solved it by adopting protocol and set delegate, it isn't an issue anymore :)

lolgear · 2020-04-24T06:09:57Z

@lucasderraugh Could we merge it?

lolgear · 2020-05-27T16:23:03Z

@lucasderraugh Any updates?

lucasderraugh · 2020-05-27T17:06:48Z

Sorry @lolgear, I have been trying to finish a project whose deadline is at the end of the month. I’ll have more time in June to work on GitUp again.

lolgear · 2020-07-06T07:38:36Z

@lucasderraugh
I hope that you finished project before deadline and everything is smooth now :)
Do you have time to check PR and merge it if it is ok?
Also, I hope #573 could be merged too.

lucasderraugh · 2020-07-06T13:51:45Z

Yup, I started looking at some PRs last night. I’ll go through some of yours later this week.

lucasderraugh · 2020-07-19T17:25:20Z

Alrighty, I’ll go through your stuff today :)

lucasderraugh

I'm trying to figure out if it should avoid being first responder or not. I generally like to hit tab to get down to the commit text at the bottom.

My biggest thing with this though is that it takes too long to do the diff on the commit. If I understand this correctly, we're only filtering by filenames, we aren't actually searching text in these commits. If this is the case then why do we need to recompute the diffs? If we're only going to filter by filename, then we should just do that at the table view level. We've already gone through the work of diffing everything.

lucasderraugh · 2020-07-25T23:04:59Z

GitUpKit/Views/Base.lproj/GIAdvancedCommitViewController.xib

+                                                                    <rect key="frame" x="0.0" y="0.0" width="300" height="27"/>
+                                                                    <autoresizingMask key="autoresizingMask" widthSizable="YES"/>
+                                                                    <subviews>
+                                                                        <searchField wantsLayer="YES" verticalHuggingPriority="750" textCompletion="NO" translatesAutoresizingMaskIntoConstraints="NO" id="u4y-Fx-A8g">


The search field should have placeholder text "Filter". We're not really searching any of the files.

lolgear · 2020-07-27T15:49:09Z

I guess it would be a little difficult to move to table-view level. The main problem is index path as id.
Yes, we search only by file name and it is unnecessary to compute diffs.
This technique with repository file pattern allows us not to do extra work and to keep consistency as "one point of truth".

lolgear · 2020-08-26T10:26:13Z

@lucasderraugh Any updates?
We could merge this PR in case of great experience, but, of course, it should be more "lightweight" and "domain-level".
However, I did exactly what is suggested in #34 .

lolgear · 2020-12-01T20:49:48Z

@lucasderraugh
Could we move away from xibs?
It is impossible to rebase now.

…ottom left corner.

…uring search. Tooltip has been added.

… been removed from file pattern handler.

…s been added.

…dded.

lolgear · 2020-12-01T21:48:04Z

@lucasderraugh Done.
Could you check and merge it?

lucasderraugh · 2020-12-02T15:07:39Z

@lolgear I think the last thing I ask on this PR is that this is an opt-in feature and that by default we don't show any filter bar. For 99% of cases it's not really useful for me and I don't want to clutter the interface if it's not useful to people. If you can add a user default checkbox in Preferences that will toggle whether this filter field is shown or not then I can merge this in.

lolgear · 2020-12-02T16:09:37Z

@lucasderraugh Do I need additional tab in settings for this behavior?
Is it ok if it will be done in code rather than in xib? ( Whole layout logic ).

lucasderraugh · 2020-12-02T17:54:28Z

@lucasderraugh Do I need additional tab in settings for this behavior?
Is it ok if it will be done in code rather than in xib? ( Whole layout logic ).

Let's keep the xibs and you can add it to the "General" tab for now. I'm going to move the "Appearance" based stuff into its own tab in a bit but lets keep the changes specific for this PR.

…t view option has been added.

…mit view option has been added.

lolgear · 2020-12-02T19:52:56Z

@lucasderraugh Done.
Yes, for this task it was pretty simple with "data binding" through "UserDefaultsController". Nice OS X pattern.

…ut has been fixed.

lolgear · 2020-12-05T14:09:29Z

@lucasderraugh
Could you merge it?

lucasderraugh · 2022-11-30T16:01:43Z

I'll resolve the conflicts here and retest. Apologies on this one.

NikKovIos · 2022-12-16T18:33:32Z

@lolgear could you please attach a screenshot of result?

NikKovIos · 2022-12-16T18:37:30Z

GitUpKit/Views/GIAdvancedCommitViewController.m

+  _commitButton.enabled = _commitButton.enabled && !self.searchInProgress;
+
+  if (self.searchInProgress) {
+    _commitButton.toolTip = NSLocalizedString(@"Search in progress", @"");


nit: should it be localized?

Robert-M-Muench · 2023-06-04T10:21:57Z

Can this be finished? This is the single most important feature missing when you work with a monorepo. I have about 2700 files showing up in my worktree and it's super tedious to scroll manually.

lucasderraugh · 2023-06-04T11:21:20Z

Yes, let me work on getting this in as it's on me at this point, I just lack the time to work on GitUp much these days. The reason it hasn't landed is because I don't want to allow filtering and committing at the same time as it's important to be able to know what is being committed. I think simply blocking commit if there is an active filter is the option I'd go with. There's also a consideration of disabling the feature in Settings that I'd like to introduce.

All this is to say let me target getting it in this week and if it's not merged you can poke me again.

Robert-M-Muench · 2023-06-06T07:28:32Z

I don't want to allow filtering and committing at the same time as it's important to be able to know what is being committed. I think simply blocking commit if there is an active filter is the option I'd go with.

How about adding some predefined filters like staged, changed, and untracked?

Or adding a real-time updated list that always shows what a commit would include?

And this feature should be very fast. I recognized that GU takes its time to rebuild the file list. When I switch to commit view initially it takes about 10s. When switching to another application and back, the update takes about 2s every time. That's pretty disturbing.

lucasderraugh · 2023-06-06T12:42:57Z

@Robert-M-Muench This is a text filter, we already indicate staged and unstaged via the 2 lists. The text filter is fast as we’re not querying anything new. The long loading time when transitioning is another bug that I need to address separately.

Robert-M-Muench · 2023-06-10T07:51:44Z

@lucasderraugh Not sure I understand.

In the commit view, I see my working directory list (about 45000 files), which includes about 2700 files/uncommitted local changes (untracked, changed, etc.).

How can I filter this list to show only changed files? Or untracked files with a specific pattern?

NikKovIos · 2024-09-02T08:14:56Z

GitUpKit/Extensions/NSView+Embedding.m

+  view.translatesAutoresizingMaskIntoConstraints = NO;
+
+  if (superview != nil && view != nil) {
+    if (@available(macOS 10.11, *)) {


nit: @lolgear think about deprecate low mac versions

@NikKovIos I’ll take a pass at cleaning this up on a new branch. The original author did enough on this one for me to push it over the finish line.

NikKovIos · 2024-09-02T08:15:37Z

GitUpKit/Views/GIAdvancedCommitViewController.m

@@ -25,9 +25,12 @@
 #import "GIColorView.h"
 #import "GIInterface.h"
 #import "GIWindowController.h"
+


Not needed spaces between imports

NikKovIos · 2024-09-02T08:16:15Z

GitUpKit/Views/GIAdvancedCommitViewController.m

+#pragma mark - Search
+- (void)setupSearch {
+  self.searchTextField.delegate = self;
+  self.searchTextField.placeholderString = NSLocalizedString(@"Filter files", nil);


nit: don't have localization

It’s fine to go that direction anyways, perhaps even encouraged. Makes any future translation easier.

NikKovIos · 2024-09-02T08:18:05Z

GitUpKit/Views/GIAdvancedCommitViewController.m

+  _commitButton.enabled =
+  _indexStatus.modified
+  || (self.repository.state == kGCRepositoryState_Merge)
+  || self.amendButton.state;  // Creating an empty commit is OK for a merge or when amending


don't commit anything that not involved into feature. code style is subjective.

lucasderraugh · 2024-09-02T12:13:17Z

Oh, forgot I left this one in a state where it’s on me. Let me resume fixing this branch up and addressing the commit concerns.

lucasderraugh requested changes Jul 25, 2020

View reviewed changes

lolgear requested a review from lucasderraugh August 26, 2020 10:43

lolgear added 8 commits December 2, 2020 00:01

kit: core live repository file pattern has been added.

52e896b

kit: advanced commit view controller search text field has been added.

1506148

kit: advanced commit view controller reset search has been added.

b8b03ab

kit: advanced commit view controller search field has been moved to b…

234062f

…ottom left corner.

kit: advanced commit view controller search field bugs have been fixed.

d3fa64b

kit: advanced commit view controller commit button has been disable d…

b994c24

…uring search. Tooltip has been added.

kit: advanced commit view controller commit button tooltip update has…

68dd5da

… been removed from file pattern handler.

kit: advanced commit view controller search text field placeholder ha…

5afa765

…s been added.

kit: advanced commit view controller xib search text field has been a…

3eaab1c

…dded.

lolgear force-pushed the commit_view_filter_files branch from 1468f4b to 3eaab1c Compare December 1, 2020 21:47

lolgear added 2 commits December 2, 2020 22:50

kit: preferences window controller xib commits enable filter in commi…

200cfe3

…t view option has been added.

kit: advanced commit view controller xib commits enable filter in com…

57df779

…mit view option has been added.

lolgear added 3 commits December 3, 2020 13:09

kit: extensions view embedding has been added.

9e342cb

kit: project has been updated.

63e5082

kit: advanced commit view controller index files view controller layo…

61cec06

…ut has been fixed.

lolgear changed the title ~~[Feature] Advanced Commit View: Filter files text field.~~ Feature. Advanced Commit View: Filter files text field. Oct 13, 2021

NikKovIos reviewed Dec 16, 2022

View reviewed changes

git-up deleted a comment from kamaraj-k Aug 29, 2024

NikKovIos reviewed Sep 2, 2024

View reviewed changes

Feature. Advanced Commit View: Filter files text field. #669

Are you sure you want to change the base?

Feature. Advanced Commit View: Filter files text field. #669

Conversation

lolgear commented Apr 12, 2020 • edited Loading

lolgear commented Apr 12, 2020

lolgear commented Apr 13, 2020

lucasderraugh commented Apr 14, 2020

lucasderraugh commented Apr 15, 2020 • edited Loading

lolgear commented Apr 15, 2020 • edited Loading

lucasderraugh commented Apr 15, 2020 • edited Loading

lolgear commented Apr 15, 2020

lolgear commented Apr 18, 2020

lolgear commented Apr 18, 2020

lucasderraugh commented Apr 23, 2020 • edited Loading

lolgear commented Apr 23, 2020 • edited Loading

lolgear commented Apr 24, 2020

lolgear commented May 27, 2020

lucasderraugh commented May 27, 2020

lolgear commented Jul 6, 2020

lucasderraugh commented Jul 6, 2020 • edited Loading

lucasderraugh commented Jul 19, 2020

lucasderraugh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lolgear commented Jul 27, 2020

lolgear commented Aug 26, 2020

lolgear commented Dec 1, 2020

lolgear commented Dec 1, 2020

lucasderraugh commented Dec 2, 2020

lolgear commented Dec 2, 2020 • edited Loading

lucasderraugh commented Dec 2, 2020

lolgear commented Dec 2, 2020 • edited Loading

lolgear commented Dec 5, 2020

lucasderraugh commented Nov 30, 2022

NikKovIos commented Dec 16, 2022

Choose a reason for hiding this comment

Robert-M-Muench commented Jun 4, 2023

lucasderraugh commented Jun 4, 2023

Robert-M-Muench commented Jun 6, 2023

lucasderraugh commented Jun 6, 2023

Robert-M-Muench commented Jun 10, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lucasderraugh commented Sep 2, 2024

lolgear commented Apr 12, 2020 •

edited

Loading

lucasderraugh commented Apr 15, 2020 •

edited

Loading

lolgear commented Apr 15, 2020 •

edited

Loading

lucasderraugh commented Apr 15, 2020 •

edited

Loading

lucasderraugh commented Apr 23, 2020 •

edited

Loading

lolgear commented Apr 23, 2020 •

edited

Loading

lucasderraugh commented Jul 6, 2020 •

edited

Loading

lolgear commented Dec 2, 2020 •

edited

Loading

lolgear commented Dec 2, 2020 •

edited

Loading