[CLOSED] Fixing issue #2923 - Getting mode from file extension won't always work

[core-ai-bot]

Issue by MarkMurphy
Sunday Mar 03, 2013 at 15:53 GMT
Originally opened as adobe/brackets#3029

---

Fixed issue #2923 Getting mode from file extension won't always work where some files don't have extensions like Makefiles for example.

---

MarkMurphy included the following code: https://github.com/adobe/brackets/pull/3029/commits

[core-ai-bot]

Comment by DennisKehrig
Monday Mar 04, 2013 at 17:13 GMT

---

I like this approach... certainly seems to be needed.

[core-ai-bot]

Comment by MarkMurphy
Monday Mar 04, 2013 at 17:33 GMT

---

@DennisKehrig I agree actually. I'll rename the getLanguageFromFilePath to getLanguageFromPath and fix the other issues later today.

[core-ai-bot]

Comment by DennisKehrig
Monday Mar 04, 2013 at 17:47 GMT

---

Happy to hear that! Can it be "for" instead of "from", though? The latter suggests to me that the language is magically contained in the path, whereas this really is a rather loose association.

[core-ai-bot]

Comment by MarkMurphy
Monday Mar 04, 2013 at 19:24 GMT

---

@DennisKehrig hmm, not sure I'm convinced... because the language kind of is "magically" contained in the path. That is after all how we're determining what language to use. Unless I misunderstood what you meant by "this" I also don't think that it should be a loose association. If anything it should be somewhat strict.

[core-ai-bot]

Comment by MarkMurphy
Tuesday Mar 05, 2013 at 01:17 GMT

---

Anything else I need to do before this can be merged?

[core-ai-bot]

Comment by DennisKehrig
Tuesday Mar 05, 2013 at 08:51 GMT

---

@MarkMurphy Not a blocker on my end, but let me have another shot at getLanguageForPath instead of getLanguageFromPath :)

We will at some point allow the user to set the language for a file themselves. Probably not just for a file extension, but directly for one file. At that point, the path becomes nothing but an arbitrary lookup string, whose contents have nothing to do with the language object returned.

On a different angle, I think we're not just getting something that is close to the file name - language ID from a file path, say "css" from "foo.css", but we're getting a whole language object that (eventually) describes a lot more, like the comment syntax. I can't get that "from" the file name, but I can get that "for" the file name.

Do you receive value for money or from money? Usually you buy goods with money, and that is the value. Getting value from money would be more like heating your home by burning it or using a coin as a screw driver or something - the inherent value.

Now I admit that this is a rather philosophical debate for such a small issue, but I'm having fun here ^^

[core-ai-bot]

Comment by MarkMurphy
Tuesday Mar 05, 2013 at 14:38 GMT

---

@DennisKehrig I didn't initially perceive it that way which is why I named it the way I did, it was just more intuitive in my mind that way but I see where you're coming from now however it still may be more personal preference than anything. I'm open to changing it but I'd like to have at least one other opinion on it before I act.@peterflynn what are your thoughts on this?

[core-ai-bot]

Comment by DennisKehrig
Tuesday Mar 05, 2013 at 14:43 GMT

---

@MarkMurphy Fair enough, thank you for considering it! It certainly will work fine either way.

[core-ai-bot]

Comment by MarkMurphy
Wednesday Mar 06, 2013 at 20:19 GMT

---

@DennisKehrig@peterflynn is a pretty busy dude so I'm just going to make the change. I thought about it some more and I think it makes sense to change it especially because it will make it a little more consistent with the previous version of this function. I'll push that up later today.

[core-ai-bot]

Comment by DennisKehrig
Wednesday Mar 06, 2013 at 20:30 GMT

---

@MarkMurphy Awesome, happy to hear that! :)
BTW: today I also stumbled over that bug where only passing an extension to getLanguageForFileExtension doesn't work. As you saw when you created your fix, so far we only called the function with full path names, apparently.

[core-ai-bot]

Comment by peterflynn
Thursday Mar 07, 2013 at 02:56 GMT

---

@MarkMurphy fwiw I don't have a strong opinion either way on the name. Whatever you guys decide is best.

[core-ai-bot]

Comment by MarkMurphy
Thursday Mar 07, 2013 at 03:04 GMT

---

@peterflynn Cool, well I think this one is ready to go.

[core-ai-bot]

Comment by MarkMurphy
Thursday Mar 07, 2013 at 03:13 GMT

---

@DennisKehrig Yeah I found that when I was updating calls to getLanguageFromFileExtension, I didn't really think about it at the time but yeah... fixed now.

[core-ai-bot]

Comment by DennisKehrig
Monday Mar 11, 2013 at 11:14 GMT

---

@MarkMurphy There are still some references to getLanguageForFileExtension in test\spec\Editor-test.js and test\spec\LanguageManager-test.js, can you fix those as well? I think then this is ready to go!

@peterflynn Since I'm assigned, I should be the one to merge it, correct?

[core-ai-bot]

Comment by MarkMurphy
Monday Mar 11, 2013 at 13:24 GMT

---

@DennisKehrig good catch, not sure how I missed those. All fixed.