[CLOSED] Correctly selects filename when known extension with a dot inside is used

[core-ai-bot]

Issue by zaggino
Tuesday Mar 18, 2014 at 21:54 GMT
Originally opened as adobe/brackets#7242

---

Follow up for adobe/brackets#7209 in case we wanted to get fancy :-)

This is for #7265

---

zaggino included the following code: https://github.com/adobe/brackets/pull/7242/commits

[core-ai-bot]

Comment by redmunds
Friday Mar 21, 2014 at 17:52 GMT

---

@zaggino This looks good, but it does not fix a case I was expecting.

This works for a file named test.html.erb, but not test.php.js. That's because the file extension "html.erb" is explicity defined in languages.json, but "php.js" is not. "html.erb" is explicitly defined because it has to be -- it doesn't fit the accepted convention -- the files should be named test.erb.html. We don't want to have to list every combination of "php.*" (and other server-side markup languages) in languages.json -- we should automatically handle them.

I think this can be done, without getting tricked by file names such as jquery-1.10.2.min.js, by also trying to match each known file extension part. For the test.php.js case, working backwards, it would check "js", then "php". Of course, it would also have to check "php.js" to handle the "html.erb" case. For the jquery-1.10.2.min.js case it would see "js" is known, but stop searching when it hits "min".

Does that make sense? Care to give it a try?

[core-ai-bot]

Comment by zaggino
Friday Mar 21, 2014 at 22:09 GMT

---

Sure, I want to finish this.

1. Do we want to have any combinations of valid extensions at the end or just some specific ones? e.g. php.js vs css.js

2. Do we want to allow just two or any number of extensions? e.g. php.html.css

[core-ai-bot]

Comment by redmunds
Friday Mar 21, 2014 at 22:38 GMT

---

I think allowing any number of extensions in any combination would be most flexible.

[core-ai-bot]

Comment by zaggino
Saturday Mar 22, 2014 at 01:26 GMT

---

Review again please@redmunds - added also tests for things mentioned above.

[core-ai-bot]

Comment by redmunds
Monday Mar 24, 2014 at 18:21 GMT

---

This works great!

One more thing I notice is that the project tree highlighting still only recognizes the file extension the old way while file renaming recognizes file extension the new way:

The project tree highlighting is done in ViewUtils.getFileEntryDisplay(entry). Just need to replace lastIndexOf with new API call.

[core-ai-bot]

Comment by zaggino
Monday Mar 24, 2014 at 21:36 GMT

---

Done :)

[core-ai-bot]

Comment by redmunds
Monday Mar 24, 2014 at 23:29 GMT

---

Looks great!

But, I'm seeing this JSLint unit test fail in your branch, but not in master:

"should default to the editor's indent"

Error: Expected true to be false.
    at new jasmine.ExpectationResult (file:///C:/Users/redmunds/dev/github/brackets-shell/Release/dev/test/thirdparty/jasmine-core/jasmine.js:102:32)
    at null.toBe (file:///C:/Users/redmunds/dev/github/brackets-shell/Release/dev/test/thirdparty/jasmine-core/jasmine.js:1194:29)
    at toggleJSLintResults (file:///C:/Users/redmunds/dev/github/brackets-shell/Release/dev/src/extensions/default/JSLint/unittests.js:43:57)
    at null.<anonymous> (file:///C:/Users/redmunds/dev/github/brackets-shell/Release/dev/src/extensions/default/JSLint/unittests.js:107:17)
    at jasmine.Block.execute (file:///C:/Users/redmunds/dev/github/brackets-shell/Release/dev/test/thirdparty/jasmine-core/jasmine.js:1024:15)
    at jasmine.Queue.next_ (file:///C:/Users/redmunds/dev/github/brackets-shell/Release/dev/test/thirdparty/jasmine-core/jasmine.js:1842:31)
    at onComplete (file:///C:/Users/redmunds/dev/github/brackets-shell/Release/dev/test/thirdparty/jasmine-core/jasmine.js:1838:18)
    at jasmine.WaitsForBlock.execute (file:///C:/Users/redmunds/dev/github/brackets-shell/Release/dev/test/thirdparty/jasmine-core/jasmine.js:2322:5)
    at file:///C:/Users/redmunds/dev/github/brackets-shell/Release/dev/test/thirdparty/jasmine-core/jasmine.js:2336:12

Seems like this is more related to your previous pull request. Are you seeing that failure?

[core-ai-bot]

Comment by redmunds
Monday Mar 24, 2014 at 23:36 GMT

---

Oh, it's because your change to JSLint has not been merged into this branch.

Looks good. Merging.