[CLOSED] Avoid searching binary files; tweaks to language mappings; APIs to unregister file extension mapping

[core-ai-bot]

Issue by peterflynn
Friday Mar 21, 2014 at 23:16 GMT
Originally opened as adobe/brackets#7290

---

• Recognize a wider variety of HTML templates embedded in script tags (e.g., as seen in http://forums.adobe.com/message/5796140#5796140)
• Remove generic "clike" language declaration that no files map to - seems unneeded, and clutters the dropdown in PR [CLOSED] navigate the sidebar with the keyboard #6409
• Add missing commenting syntax to Scala language
• Automatically filter out a variety of common binary file types from Find in Files (it already checks isBinary) - note that this does not affect Mac, where all binary file IO fails automatically. Helps reduce bugs like [CLOSED] Line Highlighting and Line Selection with Line Number #6091.
  • This is basically a head start on https://trello.com/c/JwRiuOwX/1139-1s-automatically-ignore-exclude-binary-files
  • I left the ProjectManager.isBinary() API in place for now, since it also excludes files from Quick Open and other getAllFiles() clients that don't yet check isBinary via LanguageManager the way Find in Files does. When we get to the real user story we should fully eliminate it, though.
• Fix incorrect code in Find in Files that only narrowly avoids creating bugs where filtered-out files are incorrectly added to search results.

---

peterflynn included the following code: https://github.com/adobe/brackets/pull/7290/commits

[core-ai-bot]

Comment by peterflynn
Monday Mar 24, 2014 at 06:04 GMT

---

@TomMalbran PR is updated - I went ahead and implemented #6873 so extensions can take over an already-registered file extension, and I tweaked LanguageManager to keep PHP working without needing the dummy "clike" language.

[core-ai-bot]

Comment by TomMalbran
Monday Mar 24, 2014 at 07:29 GMT

---

@peterflynn Awesome. Would it make sense to automatically unregister an extension/filename when registering a new language? I am thinking that if you want to register a new language with an extension already used by another language it is for a good reason. If not, it might be nice to be able to pass and array of extensions/filenames to the functions.

[core-ai-bot]

Comment by pthiess
Monday Mar 24, 2014 at 17:30 GMT

---

Hey@TomMalbran We were going to branch for our next release early next week, could you have a look at this by then?

Thanks!

[core-ai-bot]

Comment by TomMalbran
Monday Mar 24, 2014 at 19:50 GMT

---

@pthiess Sure. Code looks good, besides the 2 questions I added. I will test it later.

[core-ai-bot]

Comment by peterflynn
Tuesday Mar 25, 2014 at 01:01 GMT

---

@TomMalbran re auto-unregistering: there's something nice about an extension having to explicitly acknowledge that it knows it's overwriting a core setting -- this puts the default emphasis on not breaking core functionality, if a conflict arises by accident or after an extension is written. There's a precedent in how key binding registration works (same as this PR: by default, it fails, but there's an API to explicitly unregister the conflict first).

I think I missed where your 2nd question was posted. What was the other thing you wanted me to address?

[core-ai-bot]

Comment by TomMalbran
Tuesday Mar 25, 2014 at 01:44 GMT

---

@peterflynn Ok, makes sense. My other question/suggestion was to change the unregister APIs to recieve an array of strings or just an array. If you want to unregister multiple extensions at once, that change would make it better.

[core-ai-bot]

Comment by peterflynn
Tuesday Mar 25, 2014 at 03:47 GMT

---

@TomMalbran Ah, got it... I was just paralleling the existing addFileExtension() API, which also requires you to call one at a time. it seems weird to have just one of the two take an array... you're proposing changing both APIs? (for backwards compat that would require accepting string|Array, though)

[core-ai-bot]

Comment by TomMalbran
Tuesday Mar 25, 2014 at 03:52 GMT

---

Sure, we could change both. And is easy to make it take a string or an array. We already provide that with setLineCommentSyntax().

[core-ai-bot]

Comment by peterflynn
Thursday Mar 27, 2014 at 23:31 GMT

---

@TomMalbran Changes pushed -- let me know if good to go now (this fixes several reported crashes, so we really want it for Sprint 38).

[core-ai-bot]

Comment by TomMalbran
Friday Mar 28, 2014 at 05:17 GMT

---

@peterflynn Thanks for the API updates, it works great. I was able to easily update my fonts viewer extensions with just a couple of extra lines, which is nice.

Just one final thing. Do you think that we should add even more extensions (since is easy to remove them) like eot, swf, some general office strings like xls, xlsx, doc, docx, etc. I know that there are lots that we can add, but I think that users might have some of this files in their projects. Or should we just wait and see what users might have and keep updating it as the time passes? I would still definitely still add the the first 2. And should we combine the audio language with the binary one?

Code looks great and test are passing, so feel free to merge it after what you decide to do with the extra extensions.

[core-ai-bot]

Comment by peterflynn
Friday Mar 28, 2014 at 21:28 GMT

---

@TomMalbran Update pushed: fixes the typo nit and adds a bunch more binary file extensions (including all the ones you suggested).

I've left audio separate for now so we don't have to worry about any potential backwards-compatibility issues.

[core-ai-bot]

Comment by TomMalbran
Friday Mar 28, 2014 at 21:36 GMT

---

Awesome. Merging