[TomMalbran]

Added 2 special cases making it possible to move the second to last line to the last position and to move the last line up without shrinking the editor.

Currently it would be possible to move the last line to end of file or the line before the end of file, but isn't possible to move before the first line since the TexRange doesn't update to reduce the first line of the inline editor. Anyway, this makes it work better than before and maybe with a new implementation of inline editors, we could remove all the restriction and special cases on inline editors.

[peterflynn]

We should definitely add unit tests for these cases too. See usage of makeEditorWithRange() in EditorCommandHandlers-test for examples of testing edit commands within inline editors.

[TomMalbran]

Sure, I will add some tests here.

[peterflynn]

Thanks!

[TomMalbran]

I started working on the tests, but makeEditorWithRange doesn't work for all the cases, since the checks if it is working in an inline editor or not by checking if EditorManager.getFocusedInlineWidget() is not null, and with makeEditorWithRange it will always return null.

I also tried to see if it worked without any special cases, and it does, so even fixing that, these wouldn't make good test cases. Is there another way of really testing an inline editor without creating a window?

[TomMalbran]

I ended creating a window with an inline editor to do the tests, and used the same window for all the tests, so it doesn't make the tests that much slower. If there is a better alternative, I'll change the tests.

[jasonsanjose]

Assigned to me. Tagging bug #3130.

[jasonsanjose]

Add this.category = "integration"; to this suite so that it runs in the integration suite instead of the unit test suite. It's our hack to keep the unit test suite running fast. All of our tests that open a Brackets window are marked as integration.

[TomMalbran]

Will do. I knew about this, but didn't knew it could be added to internal describes or if it would be needed, since the tests are still fast, and the Extension Dialog tests uses windows and are unit tests.

[jasonsanjose]

Initial review complete

[TomMalbran]

I added the _codeMirror.doc.historySize() here too since is a no-op edit too, but since there was a editor.document.setText after the last test, it has to be 1 instead of 0.

[jasonsanjose]

I see. To make this consistent and expect zero, I'd rather change the afterEach() to close all documents in the working set. You could then change beforeEach to keep the same window open, but always re-create the working set and open the inline widget. Sound good?

[TomMalbran]

That is ok too, I did that for the Editor options handlers tests so I can grab some code from there.

[TomMalbran]

@jasonsanjose Fixes after initial review pushed.

[jasonsanjose]

@TomMalbran one more suggestion on the unit tests before merging.

[TomMalbran]

@jasonsanjose Fix pushed. I used the same solution as in the CommandOptionHandlers tests to close the documents.

[jasonsanjose]

Looks good. Merging.