[CLOSED] Integrate latest CodeMirror for sprint 36

[core-ai-bot]

Issue by njx
Wednesday Dec 18, 2013 at 23:56 GMT
Originally opened as adobe/brackets#6268

---

Update: Removed "[discussion only]" tag since we think we've addressed the issues and could merge this.

In the latest CM (as of codemirror/codemirror5@d13582f), the CSS mode has been refactored substantially. The good news is that LESS is now supported by CSS mode, meaning that we can much more easily make basic code hinting work for LESS files (similar to what we've already done for SCSS files). The less good news is that the internals of the mode have changed significantly. In particular:

• There's no longer a stack member on the state object, but instead a context member which seems to fulfill the same role (using a linked list instead of a stack).
• Some of the token types appear to have changed (specifically, propertyValue now seems to be prop).

This PR contains a quick attempt to make the existing CSS code hinting work properly with the changes to CSS mode. It also hooks up LESS files to the CSS mode (instead of the old LESS mode) and attempts to turn on code hinting in those files.

With these changes, it looks like almost all the existing CSS code hinting unit tests pass, except for one in CSS Context Info. However, I haven't done much manual testing beyond verifying that basic property and value hints seem to work in CSS and LESS files. Also, this was based on only a very cursory understanding of the existing CSSUtils code and of the changes to the CM CSS mode, so it's quite likely that we need to do more work and testing to make sure everything is working properly.

@RaymondLim - would you mind taking a look at the code changes and seeing what you think? I'm wondering if we need to add a story for this (to ensure we get adequate testing, etc.), which might mean pushing it to the next sprint.

Additionally, there were a number of other smaller changes since our last CM merge that are probably worth testing around. From looking at the commit log, these are the areas we'd want to do some sanity testing around:

• Inline editor line hiding (esp at beginning/end of doc) - make fix first
• LESS - using CSS mode now! - include that and test
• XML-mode
• Auto-closing of HTML tags
• Look for gutter issues
• Hit up arrow while selection exists
• Scroll into view
• newline and indent
• electric chars
• CSS mode changes, incl. tokenization (test quick edit, quick open symbol, etc.)
• JS mode - indentation around for / forEach
• Check undo/redo/dirty bit esp. around quick typing

Finally, this PR also contains the small update required by #6204.

---

njx included the following code: https://github.com/adobe/brackets/pull/6268/commits

[core-ai-bot]

Comment by njx
Thursday Dec 19, 2013 at 00:19 GMT

---

Just realized that codemirror/codemirror5@fad3f49, where Marijn updated his own CSS code hinting add-on to work with the new mode, might give us clues as to the right way to fix our code hinting. In particular, it looks like he uses token.state.state instead of token.state.context.type (seems like the two might be equivalent). You probably still need to use context.prev.type to get the previous state though.

[core-ai-bot]

Comment by njx
Thursday Dec 19, 2013 at 00:26 GMT

---

Also, I just realized that I don't really know what the difference is between token.type and token.state.state (or token.state.context.type). If the two are always equivalent, maybe we should just be checking token.type?

[core-ai-bot]

Comment by RaymondLim
Friday Dec 20, 2013 at 19:15 GMT

---

@njx I'm getting the following unit tests failure on Windows.

• Units suite
  • 1 failure in CSS Context (fixed)
  • 10 failures in EditorCommandHandler (fixed)
• Integration suite
  • 4 failures in EditorCommandHandler (fixed)
• Extensions suite
  • 1 failure in Bower-install
  • 1 failure in HTML Code Hints (temporarily disable the failing test since we cannot integrate the recent fix from CM yet.)
  • 1 or 2 in JS Code Hints (no failure if running only JS Code Hints unit tests)
  • 9 failures in URL Hinting (fixed)

Update: I'll be investigating CSS Context, HTML Code HInts and URL Hinting failures.

[core-ai-bot]

Comment by njx
Friday Dec 20, 2013 at 19:17 GMT

---

Good to know. I didn't run all the unit tests myself so it's not too surprising there are other failures. I have a task on the Sprint 36 CM Merge card to look into these, but I think we're waiting until Jonathan makes a call on whether to try to pull this work in. Until then, maybe we should just put this on hold.

[core-ai-bot]

Comment by njx
Tuesday Jan 07, 2014 at 01:35 GMT

---

Reviewed the most recent commit and added some comments. Let me know when there are more changes you want me to review.

[core-ai-bot]

Comment by RaymondLim
Tuesday Jan 07, 2014 at 18:00 GMT

---

I think I've fixed all unit test failures. The remaining failures -- 1 in Bower-install and 1 or 2 in JS code hints -- have nothing to do with CM update and can be reproduced in master as well. I filed an issue for the HTML Code hints unit test failure and Marijn already fixed it in codemirror/codemirror5@08d5cab, but I don't think we can include the fix in this pull request. So I just disable the unit test for now.

[core-ai-bot]

Comment by njx
Wednesday Jan 08, 2014 at 01:34 GMT

---

Makes sense. However, we should file a bug on the case related to the disabled unit test. It looks like it causes an obvious regression in the code coloring (as in the bug you filed), but I wasn't able to actually reproduce any problems with code hints in that case. It's a bit of a bummer to ship with the code-coloring regression, but we can release note it.

I did notice that the fix for the issue is only in the v4 branch of CodeMirror for some reason - I left him a comment asking about that.

[core-ai-bot]

Comment by njx
Wednesday Jan 08, 2014 at 01:35 GMT

---

Merging.

[core-ai-bot]

Comment by njx
Wednesday Jan 08, 2014 at 01:39 GMT

---

Filed #6400 on the regression above.