[CLOSED] Find in Files Improvements (Part 4: Sorting)

[core-ai-bot]

Issue by TomMalbran
Sunday Jan 19, 2014 at 02:27 GMT
Originally opened as adobe/brackets#6585

---

The Find in Files Improvements are back with part 4.

With this PR the files in the Find in Files results are sorted before showing the results. It will always show first the selected file and the rest after sorted similar to how they are displayed in the project tree in Windows.

This fixes issue #4933.

---

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

[core-ai-bot]

Comment by TomMalbran
Sunday Feb 23, 2014 at 21:12 GMT

---

@JeffryBooher Any updates here? I would like them to be merged for the next release so I can continue with the next one. This also fixes a bug where the results can change order while updating the files.

[core-ai-bot]

Comment by JeffryBooher
Monday Feb 24, 2014 at 02:49 GMT

---

Apologies@TomMalbran this must have gotten lost in the shuffle. I'll review it and pull it in sometime this week.

[core-ai-bot]

Comment by TomMalbran
Monday Feb 24, 2014 at 02:58 GMT

---

Great. Thanks

[core-ai-bot]

Comment by JeffryBooher
Wednesday Feb 26, 2014 at 22:28 GMT

---

@TomMalbran done with initial review. Just a few minor code cleanup items to work on before I test it.

[core-ai-bot]

Comment by TomMalbran
Thursday Feb 27, 2014 at 00:12 GMT

---

@JeffryBooher Thanks for the review. Changed pushed.

[core-ai-bot]

Comment by JeffryBooher
Friday Feb 28, 2014 at 18:56 GMT

---

@TomMalbran why do the path headers no longer have the full path? It's relative to the project root now. If I compare Sprint 36 to this PR, FiF says "css.json - src/extensions/default/WebPlatformDocs/" vs. "extensions/default/WebPlatformDocs". I think it's ok but I didn't see a mandate or a note so I'm curious if it was accidental or did you intend for that to happen. I like that it saves space -- especially useful for bigger projects with a lot of subfolders but we should be careful that this was intentional and wasn't broken by accident.

[core-ai-bot]

Comment by peterflynn
Friday Feb 28, 2014 at 19:37 GMT

---

@JeffryBooher Afaik, the headers in the search results have always shown project-relative paths. That's how it works for me on Sprint 36 master currently. Do you have local changes on your machine?

Or might you have had a different project root open in the two cases you tested? (Neither path example above looks like a full, absolute path to me).

[core-ai-bot]

Comment by redmunds
Friday Feb 28, 2014 at 20:09 GMT

---

A case where full paths maybe used to be shown is when you have a file open that is outside your current project since Working Set files are also searched. I tested in master and I'm not seeing any path for that case -- seems like full path should be shown.

[core-ai-bot]

Comment by JeffryBooher
Friday Feb 28, 2014 at 23:04 GMT

---

@peterflynn the only thing i'm doing is switching between master and pr/6585. There may be some changes in master that are not in pr/6585, however. But the project is basically the brackets source tree. When I get results for master it says "src/..." but for pr/6585 the headers start with the folders below "src" so "thirdparty/jquery/..."

@TomMalbran you might want to merge master into this branch as I always need to rejigger the submodules afterwards but primarily so we can see what's different between your branch and master.

[core-ai-bot]

Comment by TomMalbran
Friday Feb 28, 2014 at 23:07 GMT

---

Sure will merge with master and check that. This PR is a bit old, but since it can still be merged I haven't merged it with master.

[core-ai-bot]

Comment by TomMalbran
Friday Feb 28, 2014 at 23:26 GMT

---

@JeffryBooher Just merged with master. There is no code in this PR that changes how the results are displayed, besides sorting the files.

[core-ai-bot]

Comment by peterflynn
Saturday Mar 01, 2014 at 01:36 GMT

---

@JeffryBooher What folder is the root of your project when you're testing? src, or the parent folders of src, or something else? It's odd because the paths aren't absolute in either case, so it sounds like they're both being turned into relative paths, just relative to different things. And if not the project root, I dunno what the reference point would be...

The code that makes it relative is on line 372 -- the call to ProjectManager.makeProjectRelativeIfPossible() -- which is totally unchanged in this patch...

[core-ai-bot]

Comment by JeffryBooher
Monday Mar 03, 2014 at 21:30 GMT

---

hey@TomMalbran is there anyway that we can add a unit test for this to make sure the list is sorted?

[core-ai-bot]

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

---

@JeffryBooher We currently don't have any FindInFiles test, so not sure how to make them.

I still found a small bug. Since this places the currently selected file, the files order changes when this file the current file changes, which happens when you switch files and start editing which changes the results. Not sure if I should remove this, or save the current file at the start of the search and always use that one.

[core-ai-bot]

Comment by JeffryBooher
Tuesday Mar 04, 2014 at 00:08 GMT

---

got it. merging...