-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Make mode string in statusbar more presentable #1923
Changes from all commits
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 |
---|---|---|
|
@@ -180,12 +180,59 @@ define(function (require, exports, module) { | |
// hide on init | ||
hide(); | ||
} | ||
|
||
function getModeDisplayString(mode) { | ||
// mode is either a string or an object with a name property string | ||
var s = (typeof mode === "string") ? mode : mode.name, | ||
slash, | ||
result; | ||
|
||
s = s.toLowerCase(); | ||
|
||
// Handle special cases | ||
if (s === "javascript") { | ||
return "JavaScript"; | ||
} else if (s === "html") { | ||
return "HTML"; | ||
} else if (s === "css") { | ||
return "CSS"; | ||
} else if (s === "less") { | ||
return "LESS"; | ||
} else if (s === "text/plain") { | ||
return "Text"; | ||
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. Seems like this one at least should probably be localized... 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. Nice catch! 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, these do not need to be localized. The "Text" string is a simple reformatting of the "text/plain" mimetype that is not localized. 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 guess the question is whether users will see it that way. "Text" is a generic word, not a proper noun like the other strings being used here... so normally it would be translated. Since we're no longer showing the full mimetype string, will users get that this label originates from one? 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. That's a huge can o' worms! If we localize that string, we have to localize them all. Can we go with this for now and then deal with it if it comes up? 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 think this is the only string we need to translate, but I'm ok to leave it as is for now. |
||
} else if (s === "php") { | ||
return "PHP"; | ||
} else if (s === "xml") { | ||
return "XML"; | ||
} | ||
|
||
// Generic case | ||
|
||
// Strip "text/" or "application/" from beginning | ||
s = s.replace(/^(text\/|application\/)/, ""); | ||
|
||
// Strip "x-" from beginning | ||
s = s.replace(/^x-/, ""); | ||
|
||
// Strip any remaining "/" sections from end | ||
slash = s.indexOf("/"); | ||
if (slash !== -1) { | ||
s = s.substr(0, slash); | ||
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. Do we still want to keep those modes that have mime type like text/x-xxx to be X-xxx? eg. text/x-c++src or x-java or x-csharp 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. Good suggestion. I also stripped "x-" from beginning. Note that CodeMirror sends a mode of "text/plain" for most files, so this doesn't impact most files used on web sites. |
||
} | ||
|
||
// Uppercase first char and rest is (already) lowercase | ||
result = s[0].toUpperCase(); | ||
result += s.substr(1); | ||
|
||
return result; | ||
} | ||
|
||
exports.init = init; | ||
exports.showBusyIndicator = showBusyIndicator; | ||
exports.hideBusyIndicator = hideBusyIndicator; | ||
exports.addIndicator = addIndicator; | ||
exports.updateIndicator = updateIndicator; | ||
exports.getModeDisplayString = getModeDisplayString; | ||
exports.hide = hide; | ||
exports.show = show; | ||
}); |
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.
We should also add a special case for XMLfiles. Currently, it shows up as Xml.
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.
Do PHP and LESS have a similar problem?
Maybe we should just do a quick run-through of all the "officially" supported filetypes in EditorUtils and make sure the output is sane for all those cases?
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.
No for PHP since it shows up as HTML.
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.
I think that will change once #1825 is fixed though.
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.
@peterflynn .less files now show up as "Less". Let me know if you think this should be "LESS".
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.
It's normally written in all caps (see http://lesscss.org/), so that would be ideal.
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.
I added special cases for LESS and PHP.
Note to see PHP, you need to add a PHP block of code something like , make sure file is in the working set, place the IP inside the PHP block, switch to another file and then switch back.