Win8: lineComment and blockComment not working for c/c++ files

[antoniofagardo]

When trying to use the shortcut [Ctrl + /] for commenting 1 line of code in a *.cpp file, the lineComment is not working and the following message is displayed on the console:
called LanguageManager._getLanguageForMode with a mode for which no language has been registered: clike

The exact same message appear when I try to use the blockComment.

Tested on Brackets Sprint 38 version expérimentale 0.38.0-12606 (release 4df8afd) without any extensions

[TomMalbran]

@peterflynn It seems that the PR #7290 that removed the clike language, broke commenting for several languages including: c, ccp, csharp, java and scala. It might be even broken for more languages. So we might need to add hacks for those languages or fix #7345.

[peterflynn]

@TomMalbran That's very weird... I definitely tested those languages (hence my comment saying they didn't seem affected). Not sure how that got mixed up.

#7345 is a bunch to bite off since it involves going through architectural discussions with CM, etc. Probably best to just repeat the hack used for PHP for now. I can put up a fix tomorrow unless you want it...

[TomMalbran]

@peterflynn Yes, I thought those worked too. Actually, I think is a bigger issue than the clike one. Every language where the mode is defined as an array is broken. VB and Bash are also broken. So maybe some recent change in the Language Manager broke all these languages?

[peterflynn]

Interesting -- I tested an old Sprint 31 build and toggle comment works in Bash even though it was defined with an array back then too.

In the current code, the module-level _setLanguageForMode() is getting called with the MIME string which seems wrong. We can't necessarily just call that with the base mode (since only one can win), but at the least it seems like Language._setLanguageForMode() should get called to map the base mode to the own language. But that doesn't explain why Sprint 31 worked -- it seems to do everything the same way as master. Will look more tomorrow...

[peterflynn]

Hmm, so Bash toggle-comment was working in Sprint 37 but broke in Sprint 38. I wonder if CMv4 changed something about the mode information we get back...

[peterflynn]

...or it could be the fact that the CMv4 landing changed the toggle-comment code from using getLanguageForSelection() (which calls getModeForSelection()) to using getModeForRange() -- which has distinctly different semantics.

CC @njx for thoughts on that

[TomMalbran]

There is only one language defined for shell and vb. So I just tested changing the mode to just shell and vb and it worked fine. The clike languages might need the hack.

[TomMalbran]

Maybe as a quick fix, instead of manually defining the modes for the languages, maybe we could add a new property to the languages that require it with the real mode. Then we can just iterate through all the languages and define the new modes for those that are required.

[njx]

It's quite likely that it's due to the changes I made while upgrading to cmv4. I thought I had retained the existing semantics for the single-selection case, but it's possible that I broke something - the unit tests pass, but perhaps there are cases that weren't being tested.

[njx]

I see - there's logic around the mixed mode case that is only in getModeForSelection() and not in getModeForRange(). I haven't looked into the bug being discussed here, but if it has to do with mixed-mode, I can see how that would have gotten broken. I'm not sure why I didn't move that logic into getModeForRange() - it was probably just due to not understanding it well enough.

[bchintx]

Marking for Release 40 to @njx

[njx]

@TomMalbran Not sure what you mean that every language where the mode is defined as an array is broken - SCSS, which is defined as ["css", "text/x-scss"], seems to work fine.

Are any languages besides bash and c++/etc. broken?

[peterflynn]

@njx SCSS works fine because there's a specific hack at the bottom of LanguageManager for it -- which shouldn't be needed because unlike "php" & "htmlmixed," it's not a true mixed mode. So that hack is purely a workaround for this bug.

Re getModeForSelection() vs. getModeForRange() -- I think it's actually the not mixed case (the more common situation) where things differ. getModeForRange() returns the result of TokenUtils.getModeAt(), while getModeForSelection() returns _codeMirror.getOption("mode").

[peterflynn]

It seems to work as a fix to do self._setLanguageForMode(mode, self); in Language._loadAndSetMode().

We still need specific hacks for true mixed-modes, like PHP & HTML... but those cases are already fixed. We could clean that up via Tom's suggestion but since it's just those two existing cases I think it's not needed (better to wait for the full #7345 cleanup).

At some point we should also clarify all the APIs in LanguageManager that take or return modes to make it much more crisp which allow mimetypes and which only allow base mode names.

[peterflynn]

Ah, but @njx pointed out that getModeForRange() is currently only used by EditorCommandHandlers, so perhaps we could safely change it to behave the same way as getModeForSelection()... (Which I think means returning the mime name (or the whole config object?), instead of just the base mode name, in the non-mixed-range case).

Though it still seems wrong that doing e.g. cppLanguage.getLanguageForMode("clike") doesn't return cppLanguage. So perhaps we should do both fixes?

[njx]

Yes, I specifically introduced getModeForRange() to make it so that the multiple-selection cases in commenting could look at each individual subselection. Is the issue that TokenUtils.getModeAt() returns a {name, mode} whereas CM.getOption("mode") just returns the mode, and in the logic in EditorCommandHandlers we look at the name first and pass that to getLanguageForMode()? If so, maybe we could just fix this in the comment code by doing mode.name ? mode.mode : mode.

[njx]

That didn't fix it, so I guess I still don't really understand what's going on.

[njx]

Oh, I guess I understood it backwards. Does CM.getOption("mode") usually just return a string?

I have a fix in getModeForRange() that I think preserves the current behavior for the case where it's called from getModeForSelection() but fixes it for the case where it's called directly (as from EditorCommandHandlers). I don't really like the fix because it's not semantically clear (because I'm still not totally clear on how the language/mode stuff works), but @peterflynn maybe you can take a look at it to see if it seems to be in the ballpark: master...nj/issue-7764

[peterflynn]

FBNC - @TomMalbran can you confirm?

[TomMalbran]

Commenting in c, cpp, csharp, vb, and bash languages is now working fine.

Closing