search

brackets-archive / bracketsIssues

Archive of issues in brackets.
0 stars 0 forks source link

[CLOSED] Refactors the search results to be able to use paging in Replace All #6392

Open core-ai-bot opened 3 years ago

core-ai-bot commented 3 years ago

Issue by TomMalbran Monday Mar 03, 2014 at 03:50 GMT Originally opened as https://github.com/adobe/brackets/pull/7059

I moved most of the code that handles the Search Results bottom panel display into a new SearchResults class, and I used it in FindInFiles and ReplaceAll. This made it possible to have pagination in ReplaceAll.

I included the files sorting here too, but using it in slightly different way.

TomMalbran included the following code: https://github.com/adobe/brackets/pull/7059/commits

core-ai-bot commented 3 years ago

Comment by MarcelGerber Monday Mar 03, 2014 at 13:20 GMT

Currently in Replace All, when you tick the Toggle All checkbox, all results on all pages get toggled. It might be nice to make a way to just toggle the current pages' results.

And another cool thing would be to have an info text like 2999 of 3000 results will be replaced (counts the checkboxes).

core-ai-bot commented 3 years ago

Comment by TomMalbran Monday Mar 03, 2014 at 21:38 GMT

@SAPlayer Nice ideas, but no idea where to place all the stuff, the panel title would end up with too much text if we add all that.

core-ai-bot commented 3 years ago

Comment by TomMalbran Tuesday Mar 04, 2014 at 22:50 GMT

Instead of doing this it might be a lot easier to implement Replace in Files as Find in Files search where in the results we include checkboxes to select what to replace and an input and button to do the replace. After that is done, Replace All, can be done as a Replace in a single File. We would still need to increase the number of results from a single file.

core-ai-bot commented 3 years ago

Comment by JeffryBooher Tuesday Mar 04, 2014 at 23:50 GMT

@larz0 and@njx any thoughts? I find it a little strange that you click "Replace All" then you have more buttons and checkboxes to click later.

core-ai-bot commented 3 years ago

Comment by TomMalbran Tuesday Mar 04, 2014 at 23:53 GMT

I really like how it works now, since it lets you select what you want to replace and doesn't do it blindly. You can also see what will be replaced.

core-ai-bot commented 3 years ago

Comment by larz0 Tuesday Mar 04, 2014 at 23:59 GMT

I like how it works now too; I actually think it's a feature.

core-ai-bot commented 3 years ago

Comment by JeffryBooher Wednesday Mar 05, 2014 at 17:55 GMT

I think one of the contributing factors to my unease is feature discoverability. I wasn't sure how to test your stuff and I thought I was just going to start blindly replacing all occurrences when I clicked the button.

core-ai-bot commented 3 years ago

Comment by TomMalbran Wednesday Mar 05, 2014 at 18:23 GMT

Replace All works in the same was as is does in master. The only difference is that this can show over 300 results and using pagination.

Anyway, i don't think this is the right solution to add pagination to replace all. A better solution would be to just use find in files to do the search, and just add a replace button into that panel, when we want to also replace. That would also work with replace in files. Eventually, replace all can work as replace in files using a single file as the scope.

core-ai-bot commented 3 years ago

Comment by JeffryBooher Tuesday Mar 18, 2014 at 02:43 GMT

@TomMalbran can you merge master into this branch? This one looks a little stale.

core-ai-bot commented 3 years ago

Comment by TomMalbran Tuesday Mar 18, 2014 at 02:47 GMT

@JeffryBooher I think I am just going to close it. This got too complicated and there will be a better solution by implementing Replace in Files, using the Find in Files search (and just slightly changing the panel), and then implementing Replace All, using Replace in Files with one file as the scope.

I know how to do the UI for Replace in Files, but not sure how to handle the actual replace.

core-ai-bot commented 3 years ago

Comment by njx Wednesday Apr 30, 2014 at 01:05 GMT

@TomMalbran I finished the bulk of the Find Bar refactoring - you can see it in nj/unify-find-ui. I started trying to do a test-merge from tom/search-results into that branch, but it looks like this branch was from before the exclusions filters were added to FindInFiles. So it might be a little easier if we merge master into this branch first before trying to merge it into my branch.

If you have time to do that in the next couple days, that would be cool. I'm going to start thinking about the underlying replace-in-files functionality and maybe temporarily implementing it without the result list UI just to test it out. Once that seems to be working we could hook it up to the unified result list UI.

core-ai-bot commented 3 years ago

Comment by njx Wednesday Apr 30, 2014 at 01:06 GMT

(BTW, if you don't have time to merge master into this branch before I'm ready for it, I can do it myself.)

core-ai-bot commented 3 years ago

Comment by TomMalbran Wednesday Apr 30, 2014 at 03:42 GMT

I should have time on Thursday and Friday, or tomorrow depending on work. Go ahead and merge it, if you need it before I get to work on it.

core-ai-bot commented 3 years ago

Comment by njx Wednesday Apr 30, 2014 at 15:42 GMT

Cool, I'll let you know either way.