Shopify / retext-shopify

Warn about Shopify style guide violations with Retext.
Other
20 stars 18 forks source link

Handle ampersands #12

Open jeremyhansonfinger opened 8 years ago

jeremyhansonfinger commented 8 years ago

Ampersands in the ruleset are currently ignored.

wooorm commented 7 years ago

@jeremyhansonfinger What’s the issue with ampersands? Maybe I can help?!

P.S. I’m recently adding some more plugins to retext. Might be something useful in there (for example, indefinite-article, repeated-words).

jeremyhansonfinger commented 7 years ago

@wooorm This is probably pretty simple, but I can't figure out how to properly escape special characters when creating a rule and writing a test for it.

So far I've tried &, \&, and %26.

Rule:

  "&": {
    "replace": "and",
    "note": "Don't use ampersands."
  },

Test:

test('ampersand', function (t) {
    var actual;

    t.plan(2);

    actual = process('bacon & Eggs.');
    t.equal(
        actual.messages[0].ruleId,
        '&',
        '“&” violates ruleId “&”'
    );
    t.equal(
        actual.messages[0].source,
        'retext-shopify',
        'source is “retext-shopify”'
    );
});
jeremyhansonfinger commented 7 years ago

It fails with TypeError: Cannot read property 'ruleId' of undefined

wooorm commented 7 years ago

Ah, alright. So, the deal is that nlcst-search searches for words only, and & isn’t a word (it’s a symbol).

First: are you trying to ban all ampersands? Is AT&T wrong too? Or are loose ampersands wrong (&), and/or use in words like so: &c.?

AT&T and &c. are both words, so you should be able to search for those.

Loose ampersands, are as mentioned before, symbols, and those must be searched for using different techniques.

Something like:

var visit = require('unist-util-visit');
var toString = require('nlcst-to-string');
// `file` and `tree` are given to the transformer.
visit(tree, 'SymbolNode', function (node) {
  if (toString(node) === '&') {
    file.warn('Do not use ampersands', node, 'ampersand');
  }
});
jeremyhansonfinger commented 7 years ago

Right, of course. I forgot about how the tree is constructed of punctuation elements and word elements.

wooorm commented 7 years ago

Yeah that's something to keep in mind! Note that it's also possible to regex over the whole file, by coercing it to string ('file.toString'), And checking that. Then, use vfile-location to get positions from offsets.