Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UI: Refactors the code-mirror linting #4866

Merged
merged 1 commit into from
Nov 6, 2018
Merged

UI: Refactors the code-mirror linting #4866

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ui-v2/app/components/code-editor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const DEFAULTS = {
};
export default Component.extend({
settings: service('settings'),
helper: service('code-mirror'),
helper: service('code-mirror/linter'),
classNames: ['code-editor'],
syntax: '',
onchange: function(value) {
Expand Down
35 changes: 0 additions & 35 deletions ui-v2/app/initializers/ivy-codemirror.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,44 +1,9 @@
import { inject as service } from '@ember/service';
import { get } from '@ember/object';
import lint from 'consul-ui/utils/editor/lint';
const MODES = [
{
name: 'JSON',
mime: 'application/json',
mode: 'javascript',
ext: ['json', 'map'],
alias: ['json5'],
},
{
name: 'HCL',
mime: 'text/x-ruby',
mode: 'ruby',
ext: ['rb'],
alias: ['jruby', 'macruby', 'rake', 'rb', 'rbx'],
},
{ name: 'YAML', mime: 'text/x-yaml', mode: 'yaml', ext: ['yaml', 'yml'], alias: ['yml'] },
];
export function initialize(application) {
const IvyCodeMirrorComponent = application.resolveRegistration('component:ivy-codemirror');
const IvyCodeMirrorService = application.resolveRegistration('service:code-mirror');
// Make sure ivy-codemirror respects/maintains a `name=""` attribute
IvyCodeMirrorComponent.reopen({
attributeBindings: ['name'],
});
// Add some method to the code-mirror service so I don't have to have 2 services
// for dealing with codemirror
IvyCodeMirrorService.reopen({
dom: service('dom'),
modes: function() {
return MODES;
},
lint: function() {
return lint(...arguments);
},
getEditor: function(element) {
return get(this, 'dom').element('textarea + div', element).CodeMirror;
},
});
}

export default {
Expand Down
33 changes: 33 additions & 0 deletions ui-v2/app/services/code-mirror/linter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import Service, { inject as service } from '@ember/service';
import { get } from '@ember/object';
import lint from 'consul-ui/utils/editor/lint';
const MODES = [
{
name: 'JSON',
mime: 'application/json',
mode: 'javascript',
ext: ['json', 'map'],
alias: ['json5'],
},
{
name: 'HCL',
mime: 'text/x-ruby',
mode: 'ruby',
ext: ['rb'],
alias: ['jruby', 'macruby', 'rake', 'rb', 'rbx'],
},
{ name: 'YAML', mime: 'text/x-yaml', mode: 'yaml', ext: ['yaml', 'yml'], alias: ['yml'] },
];

export default Service.extend({
dom: service('dom'),
modes: function() {
return MODES;
},
lint: function() {
return lint(...arguments);
},
getEditor: function(element) {
return get(this, 'dom').element('textarea + div', element).CodeMirror;
},
});
2 changes: 1 addition & 1 deletion ui-v2/app/templates/dc/acls/tokens/-fieldsets-legacy.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
{{/if}}
<label class="type-text">
<span>Rules <a href="{{env 'CONSUL_DOCUMENTATION_URL'}}/guides/acl.html#rule-specification" rel="help noopener noreferrer" target="_blank">(HCL Format)</a></span>
{{code-editor class=(if item.error.Rules 'error') name='Rules' value=item.Rules onkeyup=(action 'change' 'Rules')}}
{{code-editor class=(if item.error.Rules 'error') name='Rules' syntax='hcl' value=item.Rules onkeyup=(action 'change' 'Rules')}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see we're doing a item.name.toLowerCase() == syntax.toLowerCase() on this syntax param. It caught me a bit by surprise given it wasn't an alias or a name (hcl) and I was wondering how it knew to select HCL. Maybe just worth making it an alias instead of doing that for readability?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean an ember alias yeah? Do you know how that would help here? I thought an alias was to point one property to another?

The toLowerCase was just good hygiene really, I took the mode specs from the code-mirror source and the properties/'syntax names' are in uppercase, and I didn't want to change that, so I thought there would always be the slight chance of confusion as to whether to say syntax="HCL" or syntax="hcl"

Actually looking at it I could at least use === not ==

Anyway, lemme know on the alias thing, maybe I've misunderstood what you meant.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I meant https://github.com/hashicorp/consul/pull/4866/files/98acdadaf1a3597bd3e36247b6e321aa3b1826c3#diff-c79b0ce21a1201a8516d52bd57f5404bR17 one of those things. I might be really confused.

Just hygienic regardless so LGTM.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, of course (sorry just realized what you meant now), you're not confused, I am!

This will end up going in post 1.4.0, but if you are interested in the background (more or less), here goes:

So, that JSON blob is taken directly from the codemirror modes.js file (I think its called that). That file is massive and contains a load of stuff I'll never need (it has ALL the different modes it supports). So I took the bits I need from it, but I want to leave it as close the original source as possible.

The HCL part of the blob is actually the 'ruby' one from codemirror, I just changed the name: but kept it uppercase to be consistent to the other names. So essentially HCL 'mode' is actually 'ruby' mode. I chose the syntax name for the attribute as it made sense to me, and I didn't want to use name as that means something else in HTML and mode didn't seem quite right as this is ruby mode..but hcl :S

To choose the syntax/mode, it just looks through the JSON blob, looking for 'HCL'.toLowerCase(). I could have just hardcoded the names in the JSON blob to be lowercase, but see above about wanting to change as little as possible.

</label>
{{#if create }}
<label class="type-text">
Expand Down
12 changes: 12 additions & 0 deletions ui-v2/tests/unit/services/code-mirror/linter-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { moduleFor, test } from 'ember-qunit';

moduleFor('service:code-mirror/linter', 'Unit | Service | code mirror/linter', {
// Specify the other units that are required for this test.
needs: ['service:dom'],
});

// Replace this with your real tests.
test('it exists', function(assert) {
let service = this.subject();
assert.ok(service);
});