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

Adds preferences that map extensions and filenames to languages. #7588

Merged
merged 2 commits into from
Apr 24, 2014

Conversation

dangoor
Copy link
Contributor

@dangoor dangoor commented Apr 21, 2014

This adds language.fileExtensions and language.fileNames preferences
that can set the language associated with the provided extensions or
filenames. These prefs are both objects.

This is a fix for #6831

This adds `language.fileExtensions` and `language.fileNames` preferences
that can set the language associated with the provided extensions or
filenames. These prefs are both objects.
@dangoor
Copy link
Contributor Author

dangoor commented Apr 21, 2014

hey @busykai, Raymond agreed to look at this but if you have time you might want to take a look since there may be a connection with your language selector.

// Constants

var EXTENSION_MAP_PREF = "language.fileExtensions",
NAME_MAP_PREF = "language.fileNames";
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest that these two should be used both in _prefState above and in the unit tests (exported with a dangling _).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@busykai
Copy link
Contributor

busykai commented Apr 22, 2014

@dangoor done with the review.

@dangoor
Copy link
Contributor Author

dangoor commented Apr 23, 2014

Review comments addressed. Ready for re-review @busykai (I did not do anything about the circular dependency. I figured that can be handled separately)

});

// Private for unit tests
exports._EXTENSION_MAP_PREF = _EXTENSION_MAP_PREF;
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: making indent of "=" at the same level as public exports will probably make it more readable.

@busykai
Copy link
Contributor

busykai commented Apr 24, 2014

Looks good! Just one minor nit comment which I'm not even sure is a worthy one.

@dangoor
Copy link
Contributor Author

dangoor commented Apr 24, 2014

@busykai great. I'll fix the nit and merge. Thanks!

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.

6 participants