search

brackets-archive / bracketsIssues

Archive of issues in brackets.
0 stars 0 forks source link

Fix issue where file extension is outputted twice when saving… #13886

Open core-ai-bot opened 3 years ago

core-ai-bot commented 3 years ago

Issue by grafluxe Tuesday Jun 06, 2017 at 13:19 GMT Originally opened as https://github.com/adobe/brackets/pull/13426

…a file created using DocumentManager.createUntitledDocument

Brackets v1.9.0 added support for creating untitled files with extensions (along with the ability to change language types in untitled files). See Brackets 13086. When using DocumentManager.createUntitledDocument(count, ext);, everything works as expected until you save. The bug is that when you save an untitled file with an extension, the filename outputted into the save-as dialog treats the untitled documents extension as part of its filename and appends the extension again. In other words, the save-as dialogs filename has the extension twice.

See: https://github.com/adobe/brackets/issues/13381

grafluxe included the following code: https://github.com/adobe/brackets/pull/13426/commits

core-ai-bot commented 3 years ago

Comment by petetnt Wednesday Jun 07, 2017 at 07:34 GMT

Hi@Grafluxe,

thanks for this great PR! The changes LGTM, but a unit test would be ace.

In https://github.com/adobe/brackets/blob/master/test/spec/DocumentCommandHandlers-test.js there's a boatload of tests regarding DocumentCommandHandlers, which can be ran from Debug -> Run Tests -> Integration -> DocumentCommandHandlers. A tests that would be something like it("should not append another extension if its already defined") would be nice to cover this case not caught before by the unit tests.

If you haven't worked with unit tests on Brackets before, feel free to ask any questions and we will be more than happy to point you to the right direction! 👍

core-ai-bot commented 3 years ago

Comment by grafluxe Wednesday Jun 07, 2017 at 17:20 GMT

Hi@petetnt,

The reason I didn't send a PR sooner, is because I spent much time trying to add a new integration test. Trust me, I really wanted to include it in my commit.

I could not figure out a way to test this functionality. The logic that I would need to test is nested inside a private member (_doSaveAs) which was not reachable/testable in my attempts. If there is a way to test it, please let me know and I'll be happy to add.

core-ai-bot commented 3 years ago

Comment by petetnt Wednesday Jun 07, 2017 at 17:25 GMT

@Grafluxe no problem, I bet we can find a way together! I have to think that about a bit but I'll get back to you ASAP. Or we can merge this and create an issue for adding the test too if the solution is something non-trivial

core-ai-bot commented 3 years ago

Comment by grafluxe Wednesday Aug 09, 2017 at 18:13 GMT

Hey@petetnt, was this branch ever merged into master?