-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Fix #4333 (Files incorrectly identified as plain text if folder's name has got the "#" char) #4379
Changes from 11 commits
43885a6
82e7986
11dbdfd
c3df4c9
b87f6a0
7551908
64bd9cb
77f2140
07850cd
37bbc55
8881093
ae48141
4b8ae42
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,7 +35,6 @@ define(function (require, exports, module) { | |
|
||
var NativeFileSystem = require("file/NativeFileSystem").NativeFileSystem, | ||
NativeFileError = require("file/NativeFileError"), | ||
LanguageManager = require("language/LanguageManager"), | ||
PerfUtils = require("utils/PerfUtils"), | ||
Dialogs = require("widgets/Dialogs"), | ||
DefaultDialogs = require("widgets/DefaultDialogs"), | ||
|
@@ -365,7 +364,39 @@ define(function (require, exports, module) { | |
function getDirectoryPath(fullPath) { | ||
return fullPath.substr(0, fullPath.lastIndexOf("/") + 1); | ||
} | ||
|
||
|
||
/** | ||
* Get the base name of a file or a directory. | ||
* @param {string} full path to a file or directory | ||
* @return {string} Returns the base name of a file or the name of a | ||
* directory | ||
*/ | ||
function getBaseName(fullPath) { | ||
fullPath = canonicalizeFolderPath(fullPath); | ||
return fullPath.substr(fullPath.lastIndexOf("/") + 1); | ||
} | ||
|
||
/** | ||
* Get the filename extension. | ||
* | ||
* @param {string} full path to a file or directory | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You need to add There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll fix a couple of other places where the param name is omitted as well. |
||
* @return {string} Returns the extension of a filename or empty string if | ||
* the argument is a directory | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Append |
||
*/ | ||
function getFilenameExtension(fullPath) { | ||
var baseName = getBaseName(fullPath), | ||
idx; | ||
|
||
idx = baseName.lastIndexOf("."); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Combine this with its declaration above just like the way you declare |
||
|
||
if (idx === -1) { | ||
return ""; | ||
} else { | ||
return baseName.substr(idx); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need to wrap this return statement with |
||
} | ||
} | ||
|
||
|
||
// Define public API | ||
exports.LINE_ENDINGS_CRLF = LINE_ENDINGS_CRLF; | ||
exports.LINE_ENDINGS_LF = LINE_ENDINGS_LF; | ||
|
@@ -386,4 +417,6 @@ define(function (require, exports, module) { | |
exports.isStaticHtmlFileExt = isStaticHtmlFileExt; | ||
exports.isServerHtmlFileExt = isServerHtmlFileExt; | ||
exports.getDirectoryPath = getDirectoryPath; | ||
exports.getBaseName = getBaseName; | ||
exports.getFilenameExtension = getFilenameExtension; | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,7 +23,7 @@ | |
|
||
|
||
/*jslint vars: true, plusplus: true, devel: true, nomen: true, indent: 4, maxerr: 50 */ | ||
/*global define, $, CodeMirror, PathUtils */ | ||
/*global define, $, CodeMirror */ | ||
|
||
/** | ||
* LanguageManager provides access to the languages supported by Brackets | ||
|
@@ -103,7 +103,8 @@ define(function (require, exports, module) { | |
|
||
// Dependencies | ||
var Async = require("utils/Async"), | ||
_defaultLanguagesJSON = require("text!language/languages.json"); | ||
_defaultLanguagesJSON = require("text!language/languages.json"), | ||
FileUtils = require("file/FileUtils"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: can you swap the order of this line with the one above? FileUtils and Async are the names of the external modules that use camel case and should be grouped together while the other one is a local variable. |
||
|
||
|
||
// State | ||
|
@@ -187,11 +188,11 @@ define(function (require, exports, module) { | |
* @return {Language} The language for the provided file type or the fallback language | ||
*/ | ||
function getLanguageForPath(path) { | ||
var fileName = PathUtils.filename(path).toLowerCase(), | ||
var fileName = FileUtils.getBaseName(path).toLowerCase(), | ||
language = _fileNameToLanguageMap[fileName], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: Please align |
||
extension, | ||
parts; | ||
|
||
// If no language was found for the file name, use the file extension instead | ||
if (!language) { | ||
// Split the file name into parts: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -117,13 +117,19 @@ define(function (require, exports, module) { | |
|
||
it("should switch to the HTML mode for files ending in .html", function () { | ||
// verify editor content | ||
var mode = LanguageManager.getLanguageForPath("file:///only/testing/the/path.html").getMode(); | ||
var mode = LanguageManager.getLanguageForPath("c:/only/testing/the/path.html").getMode(); | ||
expect(mode).toSpecifyModeNamed("text/x-brackets-html"); | ||
}); | ||
|
||
it("should switch modes even if the url has a query string", function () { | ||
it("should switch modes for UNIX absolute path", function () { | ||
// verify editor content | ||
var mode = LanguageManager.getLanguageForPath("http://only.org/testing/the/path.css?v=2").getMode(); | ||
var mode = LanguageManager.getLanguageForPath("/only/testing/the/path.css").getMode(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a hypothetical test case. LanguageManager is used strictly to operate on local files. I think it was inspired by the fact that PathUtils was used to parse the file path in the first place. |
||
expect(mode).toSpecifyModeNamed(langNames.css.mode); | ||
}); | ||
|
||
it("should switch modes for relative path", function () { | ||
// verify editor content | ||
var mode = LanguageManager.getLanguageForPath("only/testing/the/path.css").getMode(); | ||
expect(mode).toSpecifyModeNamed(langNames.css.mode); | ||
}); | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to add
fullPath
right after {string}.