Exclude files from Find in Files operations #7015

peterflynn · 2014-02-27T00:34:30Z

Implements Find in Files exclusions user story, with basic unit tests.

New FileFilters module manages the UI (picker widget & editor dialog), glob-matching APIs, and last-used filter preference.
New DropdownButton widget hoisted out from CSSInlineEditor -- this will later be reused as a filter picker UI once we support saving multiple. separate filters. Originally this story also called for a dropdown widget, so now that it's already implemented it seemed simpler to leave this change in the diff. (It also removes about 100 lines of code from the CSSInlineEditor module).
Switches search bar to use the standard ModalBar autoClose behavior, rather than its own one-off focus tracking logic.
Enhances ModalBar with two new capabilities:
- Can specify an isLockedOpen() function that keeps the bar open regardless of focus changes. Needed for existing Find in Files functionality - bar stays open while search in progress.
- Popups launched from widgets inside the bar can take focus without closing it (even when parented to <body>) if they specify an "attached-to" attribute pointing back at the widget inside the bar. Needed for newly-added picker dropdown.

…ing.

…it it to ten globs.

seach implementation or prefs yet. Changes the search bar to use ModalBar's autoClose behavior; no longer assumes its code is the only thing to ever close the bar. Adds a new API to ModalBar to allow the search bar to be 'locked open' when it would normally close, as Find in Files needs for its no-results state and when the filter editor dialog is open.

…ynn/search-exclusions * origin/rlim/search-exclusions: Fix JSLint error for extra trailing whitespaces. Switch to global view state instead of project-based one and also limit it to ten globs. Remove an incorrect assignment. Initial implementation search exclusion using project-based view setting.

Centralize code in FileFilters module. Switch prefs from single set of at most 10 globs to an MRU list of at most 10 separate filters, each of which is an unlimited set of globs. Properly update dropdown UI when editing dialog is dismissed

…se it in FileFiters picker UI also, replacing the <select> element it used before. Further enhance ModalBar to accept focus going to popup elements outside its DOM hierarchy if they point back to the ModalBar as the popup's opener.

- Automatically add ** prefix and/or suffix in most cases - Link to real docs page on wiki - Show full filter set in picker button's tooltip

peterflynn · 2014-02-27T00:34:59Z

@RaymondLim You might want to take a quick look at 6cec6e9, which is the main place where I reconciled your earlier code with my UI code.

Small starter set of unit tests for FileFilters auto-prefix/suffix behavior. Also fix whitespace JSLint glitch in unrelated unit-test file.

peterflynn · 2014-02-27T01:18:36Z

src/htmlContent/stylesheets-menu.html

-        </a>
-    </li>
-    {{/styleSheetList}}
-</ul>


DropdownButton uses item renderers now, so this template isn't needed. I thought about trying to implement everything in terms of Mustache partials, but it wouldn't be very clean to implement the 'clipping' behavior that the filter-picker item renderer needs... It didn't seem worth the effort to use Mustache for such tiny snippets.

TomMalbran · 2014-02-27T02:39:07Z

This is awesome :)

But I think we can improve the filters management UI a bit more since I didn't found it that intuitive.

I think that when you select Edit Filter... it should take you to a UI that would let you add/edit/delete any filter. That UI could be just a select with all the filters and Add New Filter and the textarea. After selecting a filter or Add New Filter, the textarea fills with the required content and the buttons change accordingly: Edit and Delete for editing and Add for adding. After clicking any button it could confirm the changes and stay in the dialog or close it, if the dialog isn't automatically closed, a Close button would be required.

An alternative could be a 2 steps UI, The first dialog opens where you select a filter and after clicking Edit a new dialog opens to edit it, or from the first dialog you can click on Delete to delete it. There would be an Add button that opens the second dialog to add a new filter.

njx · 2014-02-27T02:45:47Z

I think the intent was to keep this simple, so you don't "manage a list of filters" - we just have a history of the filters you've used. But I think what's confusing is that when you choose "Edit filter", it feels like you should be editing the current filter, but instead it adds a new filter.

I wonder if it would be better to make it so "Edit filter" really does edit the last selected filter, and there's a separate "New filter" for creating a new one.

njx · 2014-02-27T02:48:16Z

A couple other thoughts on the UI:

The first time the user sees this UI, it would make sense for it to just be a "Filter..." button (not a dropdown), so you can get right into the filter UI. Once there's at least one filter in the history, it could change to a dropdown.
The file glob docs say the user should use forward slashes on Windows. Could we just translate backslashes on Windows automatically? It seems unfriendly to require forward slashes for Windows users.

njx · 2014-02-27T02:49:50Z

Actually, another thought...what if when you hover over a filter in the list, it gets icons next to it - "x" to delete, pencil to edit? This would be similar to what we do in the Recent Project dropdown. Then we would just need a "New filter" entry at the bottom to create a new one.

TomMalbran · 2014-02-27T02:53:54Z

I see. I thought I was editing the filter I selected, but it just creates a new one.

I like @njx idea about the x and pencil. That would keep it simple enough. If I created a wrong filter, I would like to remove it from the list, or edit it.

njx · 2014-02-27T02:57:32Z

/cc @larz0 relevant to your interests

peterflynn · 2014-02-27T03:11:05Z

@TomMalbran @njx I think the idea was that most of the filter management stuff -- delete, name/rename, save explicitly -- was sliced out into a separate story so we could ship the core functionality sooner. I'd rather save things like an "x" and pencil icon for that story since it seems like we'd still prefer to wrap this sprint up sooner.

The idea of splitting Edit Filter from New Filter seems reasonable... although it does mean that the list isn't really a history list anymore since it might no longer contain the filter you just recently used (if you've edited it since then). It seems like this creates a pitfall -- it could lead to the list only ever having 1 thing in it (which the user repeatedly edits as needed), losing the benefit of having a list in the first place. Maybe one way around that would be to keep the current UI (Edit only), but don't add repeated edits as separate items to the history list -- only add something when you actually run a search using it...

Re backslashes: my only concern with that is that it'd be inconsistent with the rest of Brackets. We display paths with forward slashes everywhere in the UI, and other places that do matching (e.g. Quick Open, the preferences system) don't do this translation.

peterflynn · 2014-02-27T03:14:07Z

In the long run (i.e. user story linked above) I think we should do the larger UI Tom mentioned, since it has the benefit that you can name common filters. Right now, it's very hard to represent the arbitrary filter strings in a compact way, so allowing "friendly names" feels like a big plus. Especially since the list of filters is global across all projects...

njx · 2014-02-27T03:20:27Z

Makes sense to keep the other functionality separate.

I don't think the change you're suggesting to Edit fixes the fundamental problem, which is that "Edit" sounds like it should modify something rather than creating a new thing. I agree that if we split it into New/Edit it won't feel as much like history, but I think it actually makes it easier for the user to map what they want onto the functionality - tweaking a filter vs. creating a new one. It does make it harder to base a new filter on an existing one, though.

You're right about backslashes - I had forgotten that we don't really deal with them anywhere in the UI. I guess no one's complained so far :)

TomMalbran · 2014-02-27T03:35:58Z

If the edit will be in a different story, that is good.

I do think that Edit filter should be renamed to Add new filter since you are not editing any filter, just adding a new one. We could just complete the textarea with the last filter, but it should be clear that you are adding a filter based on that one.

On the complete UI, we could duplicate filters, and even make filters to be used only in the current project.

peterflynn · 2014-02-27T04:06:01Z

Hmm, I think we have a conceptual model mismatch going on. The original proposal for the first-pass story was: you have one filter, which you can edit, and that's it. Then we extended it by saying there's also a history of previously-used filters; but what you're editing is always the one, current filter. You're not 'making a new filter' when you modify that filter, but the history list does grow since the once-current value is now a previous value.

If that model isn't clear in this UI -- and it sounds like it isn't -- we could try to make it less ambiguous, e.g. by moving Edit to the top, adding a "History" heading to the list below it, and only adding to the history list when you execute the search... But I wonder if we should actually go back to the original plan at this point -- one filter, no list at all. That might be better than putting more effort into this temporary history/MRU list that's actually going to get replaced with a full filter-management UI in the near future.

Or a third option would be to say we shouldn't ship this at all until the full UI can be implemented...

njx · 2014-02-27T23:21:36Z

@peterflynn and I chatted today and we decided to just simplify it for now as he suggested above (one filter, no history).

larz0 · 2014-02-27T23:37:26Z

Yes I think we should simplify it for now. We could rename "Edit Filter" to "Edit Filter List" to make it clearer.

RaymondLim · 2014-02-28T00:46:56Z

src/search/FindInFiles.js

        // Hide error popup, since it hangs down low enough to make the slide-out look awkward
        $(".modal-bar .error").hide();

        this.closed = true;
-        this.modalBar.close(true, !suppressAnimation);
        EditorManager.focusEditor();


I think we should set focus back to the Find textfield instead of the editor. Even better, if the user already put some search text in textfield, do search right away with OK button. In that case, we may also consider labeling OK button with Search.

This is the code that runs when the entire search bar is hidden, so there won't be any Find textfield to focus at this point. I think mean what we do when the Edit Filter dialog is closed? In that case, I agree we should focus the Find field -- I'll see if I can fix that.

Yes, I meant when Edit Filter dialog is closed.

…tead of globmatch, for a 100x speedup. With lots of unit tests for the new glob-matching engine.

peterflynn · 2014-03-03T21:42:32Z

@JeffryBooher The performance fix is pushed up, with a big chunk of new unit tests, and I've updated the syntax docs on the wiki as well. Should be 100% ready for full review now.

JeffryBooher · 2014-03-04T00:15:07Z

@peterflynn done with initial review. I just want to play around with the wild card stuff a bit and I think we're ready to go.

JeffryBooher · 2014-03-04T23:01:19Z

@peterflynn done with testing, can you merge master into this branch so I can do the merge?

TomMalbran · 2014-03-04T23:06:39Z

src/search/FindInFiles.js

@@ -42,7 +42,8 @@
 define(function (require, exports, module) {
    "use strict";

-    var _ = require("thirdparty/lodash");
+    var _ = require("thirdparty/lodash"),
+        FileFilters = require("search/FileFilters");


Small Nit: Why not join these with the vars below?

RaymondLim · 2014-03-05T00:18:22Z

Done with my initial review.

reliably detect when filter is blank. (Also fixes potential display bugs when there are empty lines within a non-empty filter). Code review fixes: docs, whitespace nits, strings

* commit '8572b1d8': Only store the selected entry when there is something selected Removes the setInsertHintOnTab method Store the selected entry when starting a search and use it when sorting the files A better fix suggested from Randy's code review. Code review changes Rename menuEntry object Rename prefs Code review changes, change menu entry order Strip leading/trailing whitespace from extension url Fix nits, use PreferencesManager Add unit test for remembering regexp state for Find Next after search bar is closed. Also refactors out some common code from the case sensitivity test. Use state of case sensitivity toggle even when find bar isn't open Improvements to Close Others Changes after first Review New Insert Code Hints with Tab Preference Handle code completion correctly with auto close braces on. Sorts the Find in Files results Conflicts: src/search/FindInFiles.js

peterflynn · 2014-03-05T00:57:38Z

@JeffryBooher @RaymondLim @TomMalbran Changes pushed. Merged up with master as well.

TomMalbran · 2014-03-05T01:06:40Z

src/search/FileFilters.js

+    function editFilter(filter) {
+        var lastFocus = window.document.activeElement;
+
+        var globInfoURL = "https://github.com/adobe/brackets/wiki/Using-File-Filters";


Should we move this url to brackets.conifg.json?

Sounds reasonable - will do

- move URL into config.json - change DropdownButton.itemRenderer to a constructor arg, change default renderer to a member function - change DropdownButton._renderList() to a member function

peterflynn · 2014-03-05T22:29:08Z

@JeffryBooher I pushed a commit to address @TomMalbran's last few bits of feedback. I think this is ready to merge if you are.

JeffryBooher · 2014-03-06T05:25:12Z

Looks good. Merging.

Exclude files from Find in Files operations

RaymondLim and others added 10 commits February 21, 2014 11:59

Initial implementation search exclusion using project-based view sett…

12456de

…ing.

Remove an incorrect assignment.

56e70d6

Switch to global view state instead of project-based one and also lim…

d960b63

…it it to ten globs.

Fix JSLint error for extra trailing whitespaces.

f87f488

Merge remote-tracking branch 'origin' into rlim/search-exclusions

bafbe88

File exclusions usability improvements:

0068fa7

- Automatically add ** prefix and/or suffix in most cases - Link to real docs page on wiki - Show full filter set in picker button's tooltip

Fix bug that snuck into last FileFilters commit at last minute.

7a96840

Small starter set of unit tests for FileFilters auto-prefix/suffix behavior. Also fix whitespace JSLint glitch in unrelated unit-test file.

peterflynn reviewed Feb 27, 2014
View reviewed changes

RaymondLim reviewed Feb 28, 2014
View reviewed changes

Simplify file-exclusion filter syntax so we can use plain regexps ins…

f8091fe

…tead of globmatch, for a 100x speedup. With lots of unit tests for the new glob-matching engine.

TomMalbran reviewed Mar 4, 2014
View reviewed changes

peterflynn added 2 commits March 4, 2014 16:40

Fix bug when clearing filter: move trimming back to editFilter() so we

b0ea154

reliably detect when filter is blank. (Also fixes potential display bugs when there are empty lines within a non-empty filter). Code review fixes: docs, whitespace nits, strings

TomMalbran reviewed Mar 5, 2014
View reviewed changes

Code review:

0195c2f

- move URL into config.json - change DropdownButton.itemRenderer to a constructor arg, change default renderer to a member function - change DropdownButton._renderList() to a member function

JeffryBooher added a commit that referenced this pull request Mar 6, 2014

Merge pull request #7015 from adobe/pflynn/search-exclusions

58e52b9

Exclude files from Find in Files operations

JeffryBooher merged commit 58e52b9 into master Mar 6, 2014

JeffryBooher deleted the pflynn/search-exclusions branch March 6, 2014 05:25

busykai mentioned this pull request Mar 7, 2014

Switch language / syntax mode of current document #6409

Merged

core-ai-bot mentioned this pull request Aug 30, 2021

[CLOSED] Exclude files from Find in Files operations brackets-archive/bracketsIssues#6351

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Exclude files from Find in Files operations #7015

Exclude files from Find in Files operations #7015

peterflynn commented Feb 27, 2014

peterflynn commented Feb 27, 2014

peterflynn Feb 27, 2014

TomMalbran commented Feb 27, 2014

njx commented Feb 27, 2014

njx commented Feb 27, 2014

njx commented Feb 27, 2014

TomMalbran commented Feb 27, 2014

njx commented Feb 27, 2014

peterflynn commented Feb 27, 2014

peterflynn commented Feb 27, 2014

njx commented Feb 27, 2014

TomMalbran commented Feb 27, 2014

peterflynn commented Feb 27, 2014

njx commented Feb 27, 2014

larz0 commented Feb 27, 2014

RaymondLim Feb 28, 2014

peterflynn Feb 28, 2014

RaymondLim Feb 28, 2014

peterflynn commented Mar 3, 2014

JeffryBooher commented Mar 4, 2014

JeffryBooher commented Mar 4, 2014

TomMalbran Mar 4, 2014

peterflynn Mar 5, 2014

RaymondLim commented Mar 5, 2014

peterflynn commented Mar 5, 2014

TomMalbran Mar 5, 2014

peterflynn Mar 5, 2014

peterflynn commented Mar 5, 2014

JeffryBooher commented Mar 6, 2014

Exclude files from Find in Files operations #7015

Exclude files from Find in Files operations #7015

Conversation

peterflynn commented Feb 27, 2014

peterflynn commented Feb 27, 2014

Choose a reason for hiding this comment

TomMalbran commented Feb 27, 2014

njx commented Feb 27, 2014

njx commented Feb 27, 2014

njx commented Feb 27, 2014

TomMalbran commented Feb 27, 2014

njx commented Feb 27, 2014

peterflynn commented Feb 27, 2014

peterflynn commented Feb 27, 2014

njx commented Feb 27, 2014

TomMalbran commented Feb 27, 2014

peterflynn commented Feb 27, 2014

njx commented Feb 27, 2014

larz0 commented Feb 27, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peterflynn commented Mar 3, 2014

JeffryBooher commented Mar 4, 2014

JeffryBooher commented Mar 4, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RaymondLim commented Mar 5, 2014

peterflynn commented Mar 5, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peterflynn commented Mar 5, 2014

JeffryBooher commented Mar 6, 2014