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

Make mode string in statusbar more presentable #1923

Merged
merged 4 commits into from
Oct 24, 2012
Merged

Conversation

redmunds
Copy link
Contributor

This is for #1826

This fixes exception with many modes such as JSON. Mode can be either a string or an Object with a name property, but it was assumed to be a string.

Handled as a special case: JavaScript, HTML, CSS. Any others we should handle?

For remaining cases, we strip "/" and everything after it, and then capitalize what's left. Most of these are simply "Text".

slash = s.indexOf("/");

if (slash !== -1) {
s = s.substr(0, slash);
Copy link
Contributor

Choose a reason for hiding this comment

The 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

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.

@ghost ghost assigned RaymondLim Oct 24, 2012
@RaymondLim
Copy link
Contributor

Done with initial review.

@redmunds
Copy link
Contributor Author

Changes. Pushed.

s = s.substr(5);
} else if (s.indexOf("application/") === 0) {
s = s.substr(12);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

You can replace the entire if statement (and else if) with s = s.replace(/^(text\/|application\/)/, ""); Then you don't need those hard-coded numbers to extract sub strings.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This could be slower since it does a string replacement every time if a change was made or not, but I went ahead and made this change because the code is a bit cleaner :)

@RaymondLim
Copy link
Contributor

Done reviewing the additional changes.

s = s.toLowerCase();

// Handle special cases
if (s === "javascript") {
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 also add a special case for XMLfiles. Currently, it shows up as Xml.

Copy link
Member

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?

Copy link
Contributor

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.

Copy link
Member

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.

Copy link
Contributor Author

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".

Copy link
Member

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.

Copy link
Contributor Author

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.

@redmunds
Copy link
Contributor Author

Done with changes for code review comments.

RaymondLim added a commit that referenced this pull request Oct 24, 2012
Make mode string in statusbar more presentable
@RaymondLim RaymondLim merged commit 4b066af into master Oct 24, 2012
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants