Skip to content
This repository has been archived by the owner on May 25, 2019. It is now read-only.

Options not working correctly due to way directive feeds them to CodeMirror, need to be ordered correctly #61

Open
WhatFreshHellIsThis opened this issue Jul 7, 2014 · 9 comments

Comments

@WhatFreshHellIsThis
Copy link

I'm having a wierd issue with linting not working and getting an error about it (Error: Required option 'getAnnotations' missing (lint addon)) and so far no normal or usual reason for this has been found related to CodeMirror itself so I'm looking farther afield.

I've confirmed some things with the CodeMirror author and still not explanation found, it should be working.

I'm guessing using this directive shouldn't affect this since everything else seems to work fine but just thought I would check to be certain.

@douglasduteil
Copy link
Contributor

Can you put a demo there ?

@WhatFreshHellIsThis
Copy link
Author

I found a problem and I think it's likely related to my original issue but not 100% certain, this is a definite issue however with the directive.

Turns out when you use the directive, the order of the configuration options matters if you want to see lint markers in the gutter but when you're not using the directive it doesn't matter at all:

Here's what I'm seeing, this is my markup:

 <div ui-codemirror="scriptEditorOptions" ui-refresh='refreshScriptEditor' ng-model="report.script"></div>

This is the minimum configuration that triggers the problem:

//this will not show lint markers in the gutter due to the option "lint: true" appearing before the option 
//"gutters"
 $scope.scriptEditorOptions = {
                lineNumbers: true,
                mode: 'javascript',
                lint: true,
                gutters: ['CodeMirror-lint-markers']
            };

This configuration will show the markers in the gutters, note that it's exactly the same except the lint and gutters lines have been transposed so that gutters comes first:

//this *will* show lint markers in the gutter
 $scope.scriptEditorOptions = {
                lineNumbers: true,
                mode: 'javascript',               
                gutters: ['CodeMirror-lint-markers'],
                lint: true
            };

Now that seems like a potential bug in CodeMirror right? However it is not because you can transpose those same two lines in the stock CodeMirror example page for lint and there is no issue, lint markers still show, I've tested it:

 <p><textarea id="code-js">

</textarea></p>

<script>
//Lint before gutters in non-directive mode, still works
  var editor = CodeMirror.fromTextArea(document.getElementById("code-js"), {
    lineNumbers: true,
    mode: "javascript",
    lint: true,
    gutters: ["CodeMirror-lint-markers"]
  });
</script>

So it seems that something in the directives updateOptions method or surrounding code is causing CodeMirror to get confused. I'm guessing this is the same reason why I'm having other issues as initially reported but this was the first direct evidence I can find that there is a difference between using CodeMirror with and without the Angular directive. I'll continue to dig.

@WhatFreshHellIsThis
Copy link
Author

Ok, did some more digging, this is definitely an issue with this directive and a serious one at that. Normally with CodeMirror the order the options are set in does not matter at all (I've confirmed for certain) but when using the directive it matters very much. For example if using the directive you put mode: 'javascript' after the lint: true option then it gives a nasty error, but when not using the directive this is perfectly valid.

Or (only when using the Angular directive) your config has the lint: true option specified before the gutters option then there is no warning display in the gutter at all whereas this is not an issue in non directive mode.

The problem appears to be that the setOption method used by the directive to feed the options one by one is not equivalent to whatever CodeMirror does itself when it reads in all the options at once when used in non Angular Directive mode.

I'm not sure if this is a bug with this directive or with CodeMirror itself. The issue I opened there is here: codemirror/codemirror5#2674

@WhatFreshHellIsThis
Copy link
Author

Stated by @marijnh over at CodeMirror repo in the issue I linked to:
"Indeed, setting options one at a time like this will cause problems for a few options. It is also super inefficient. I'd say that this is a bug in the Angular wrapper, not in CodeMirror."

As a workaround I have played with it repeatedly changing the options around until I found the combination that seems to work in the order they are fed to the CodeMirror component.

@WhatFreshHellIsThis WhatFreshHellIsThis changed the title Anyone having issues with Linting using this directive? Options not working correctly due to way directive feeds them to CodeMirror, need to be ordered correctly Jul 10, 2014
@douglasduteil
Copy link
Contributor

Hi @WhatFreshHellIsThis. I have been following your issue with interest. I agree with @marijnh it's clearly a naive approach to just set the options one by one.

@marijnh
Copy link

marijnh commented Jul 10, 2014

If you want to stick to the setOption approach, at least follow the order of the properties in CodeMirror.defaults by doing a for/in loop over that, rather than depending on the object passed by the user. But it'd be better to build up an option object and pass that to the CodeMirror constructor, since CodeMirror can initialize more efficiently if it knows its full configuration upfront.

@douglasduteil
Copy link
Contributor

Thanks @marijnh 👍

douglasduteil added a commit that referenced this issue Jul 11, 2014
@douglasduteil
Copy link
Contributor

@WhatFreshHellIsThis Here is the result of the last update I made

@WhatFreshHellIsThis
Copy link
Author

Looks good. I reordered the options in the Plunker so that they were in the order that broke for me here and it seems to work good! I also see you got ui-codemirror-opts working so we can change from ui-codemirror instead?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants