errata-ai / Microsoft

A Vale-compatible implementation of the Microsoft Writing Style Guide.
https://github.com/errata-ai/vale
MIT License
85 stars 46 forks source link

Cannot extend Headings.yml #16

Closed amyq closed 5 years ago

amyq commented 5 years ago

I'm attempting to create a company-specific version of https://github.com/errata-ai/Microsoft/blob/master/Microsoft/Headings.yml and I'm finding that if …

… the words in microsoft/Headings.yml are treated as exceptions, but words in acquia/Headings.yml are not, unless I add microsoft.Headings = NO to _vale.ini

(The 'microsoft' style is called before the 'acquia' style in _vale.ini, if that's useful)

Here's the first 12 lines from our acquia/Headings.yml file:

extends: capitalization
message: "'%s' should use sentence-style capitalization."
level: suggestion
scope: heading
match: $sentence
exceptions:
  - Academy
  - Acquia
  - Cloud

When I disable the Microsoft file, this subheading is (correctly) allowed:

Evaluating Acquia Cloud
-----------------------
jdkato commented 5 years ago

Exceptions are only considered for the rule they're listed in -- in other words, microsoft.Headings respects the exceptions in its own file, but doesn't consider those listed in acquia.Headings.

So, if you have both microsoft.Headings and acquia.Headings enabled, the expected result is for microsoft.Headings to complain while acquia.Headings does not:

 test.md
 1:1  suggestion  'Evaluating Acquia Cloud'       microsoft.Headings 
                  should use sentence-style                          
                  capitalization.  
amyq commented 5 years ago

Ah - I assumed they would be treated as additive, since they were both extensions of the capitalization rule.

wouter-veeken commented 5 years ago

@jdkato For the spellchecker, one can reference an external .txt with exceptions. Would a similar thing work for this?

jdkato commented 5 years ago

I'm not sure that I understand the goal here.

If microsoft.Headings and acquia.Headings did indeed respect each other's exceptions, you'd always receive duplicates alerts: anytime a heading violated one rule, it'd also violate the other. Essentially, you would have implemented the same rule twice.

The same issue would exist if both microsoft.Headings and acquia.Headings referenced the same external ignore file, as suggested above.

wouter-veeken commented 5 years ago

Durr, you're right. 🤔

So I suppose the only sane solution in this scenario is to have a single catch-all check?

amyq commented 5 years ago

I think so. It's understandable, but disappointing; our corporate style starts with the Microsoft style guide, then layers industry- and company-specific style expectations atop it. We were hoping to keep importing the MS style without modifying those files, and keep our company-specific tests in a separate style. Data hygiene.

jdkato commented 5 years ago

Admittedly, I think the notion of customizing a "third-party" style (i.e., one that technically neither Vale nor the active user created) is one of Vale's weaker points at the moment. (FWIW, this is something that I've addressed in Vale Server).

However, I think the way styles interact with each other is an entirely different issue and is handled fairly well currently. There's no way that some kind of inheritance system involving intermixing of exceptions between styles could work. In addition to the duplication issue mentioned above, it's quite possible that rules extending the same point (e.g., capitalization) expect the opposite exceptions: for example, microsoft.Headings could want sentence case, while some-style.Headings wants title case.


That said, here's what I would do:

  1. In your .vale.ini file, I'd use both styles as a base:

    StylesPath = path/to/some/directory
    MinAlertLevel = suggestion
    
    [*.md]
    BasedOnStyles = Microsoft, Acquia
  2. Whenever you encounter a rule that doesn't work with the in-house style or terminology, I'd change it in the custom style:

    StylesPath = path/to/some/directory
    MinAlertLevel = suggestion
    
    [*.md]
    BasedOnStyles = Microsoft, Acquia
    
    # Disabled in favor of Acquia.Headings, which has an exception's list
    # tailored to Acquia's content.
    Microsoft.Headings = NO

    In practice, you should only need to do this when either your custom style diverges from Microsoft or your exceptions (conditional and capitalization rules) do.

This ensures that you have "data hygiene" both ways: Microsoft doesn't care about Acquia's exceptions or non-MS guidelines, and Acquia only cares about its guidelines that aren't already covered (adequately) in Microsoft.

jdkato commented 5 years ago

I'm going to close this for now -- but if anyone has any ideas for improving my suggested workflow, feel free to comment further.