Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Avoid searching binary files; tweaks to language mappings; APIs to unregister file extension mapping #7290

Merged
merged 4 commits into from
Mar 28, 2014
Merged

Avoid searching binary files; tweaks to language mappings; APIs to unregister file extension mapping #7290

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
247d711
A few small tweaks to language mappings:
peterflynn Mar 21, 2014
78ac6ea
- Ability to remove file extension/name mappings from existing Language
peterflynn Mar 24, 2014
e9ad8a0
Code review: change Language add/remove extension & filename APIs to …
peterflynn Mar 27, 2014
e013ca4
Add some more common binary types: Node native extensions, Flash, EOT
peterflynn Mar 28, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/language/LanguageManager.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -770,11 +770,11 @@ define(function (require, exports, module) {
_patchCodeMirror();

// Define a custom MIME mode here instead of putting it directly into languages.json
// because JSON files must not contain regular expressions. Also, all other modes so
// because JSON files can't contain regular expressions. Also, all other modes so
// far were strings, so we spare us the trouble of allowing more complex mode values.
CodeMirror.defineMIME("text/x-brackets-html", {
"name": "htmlmixed",
"scriptTypes": [{"matches": /\/x-handlebars-template|\/x-mustache/i,
"scriptTypes": [{"matches": /\/x-handlebars|\/x-mustache|^text\/html$/i,
"mode": null}]
});

Expand Down
18 changes: 10 additions & 8 deletions src/language/languages.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,13 +120,6 @@
"lineComment": ["//"]
},

"clike": {
"name": "clike",
"mode": "clike",
"blockComment": ["/*", "*/"],
"lineComment": ["//", "#"]
},

"java": {
"name": "Java",
"mode": ["clike", "text/x-java"],
Expand All @@ -138,7 +131,9 @@
"scala": {
"name": "Scala",
"mode": ["clike", "text/x-scala"],
"fileExtensions": ["scala", "sbt"]
"fileExtensions": ["scala", "sbt"],
"blockComment": ["/*", "*/"],
"lineComment": ["//"]
},

"coffeescript": {
Expand Down Expand Up @@ -242,5 +237,12 @@
"name": "Audio",
"fileExtensions": ["mp3", "wav", "aif", "aiff", "ogg"],
"isBinary": true
},

"binary": {
"name": "Other Binary",
"fileExtensions": ["svgz", "jsz", "zip", "gz", "htmz", "htmlz", "rar", "tar", "exe", "bin", "dll", "pdb", "lib", "obj", "so", "a", "dylib",
"pdf", "psd", "ai", "tif", "tiff", "mpg", "mpeg", "avi", "flv", "mp4", "ttf", "otf", "woff"],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this break extensions like mine that creates a custom viewer for fonts since now some of the extensions are in this unused language? https://github.com/TomMalbran/brackets-fonts-viewer

My extension defines a font language, and uses it to register a custom font viewer. We could have other extensions doing the same for pdf, zip files and others.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, good point - so we should hold off on merging this part until #6873 is implemented

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or we could at least split them and give them better languages and not put them all in the same bag. But we could still run into the same issue.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should think about grouping them a bit more in order to make some more custom viewers available. Maybe think of these:

  • video
  • audio
  • fonts

And why don't we add .tiff or such to the image group? The image viewer should still work, so there are no problems, are they?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don`t think that we need separate languages, I would just put them all together. A custom viewer extension can just remove the file extensions from this list and create the language. Also, we can't even open audio or video files in Brackets, so doesn't make sense to make languages that we are not using.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think if we're not doing any special handling of them we should just lump them all together for now. As @TomMalbran says, if an extension wants to provide support for a bunch of font type, or video, or whatnot, it can register a new language and take over those file extensions itself.

I'm not sure why we even break out audio separately -- but just in case extensions are relying on it, I'd like to leave it as-is for now.

@SAplayer The reason .tiff isn't listed under images is because Brackets doesn't support it -- our image-preview UI can't render it, and it's not a format used when coding CSS/HTML (for the same reason).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we can break extensions that use any of the new file extensions added here, we could just also break extensions that use the audio language. I don't think that there should be any with it since you can't do much with it. Can you check that?

"isBinary": true
}
}
4 changes: 3 additions & 1 deletion src/search/FindInFiles.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -751,7 +751,9 @@ define(function (require, exports, module) {
var inWorkingSet = DocumentManager.getWorkingSet().some(function (wsFile) {
return wsFile.fullPath === file.fullPath;
});
return inWorkingSet;
if (!inWorkingSet) {
return false;
}
}
}
// In the initial search, this is passed as a getAllFiles() filter
Expand Down