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.

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

Add ability to customize/override rule message text #400

Closed
bluetidepro opened this issue Oct 1, 2015 · 23 comments
Closed

Add ability to customize/override rule message text #400

bluetidepro opened this issue Oct 1, 2015 · 23 comments
Labels
status: ready to implement is ready to be worked on by someone

Comments

@bluetidepro
Copy link

It would be really nice if there was an easy way to customize (or override) the default rule text for each rule when you declare the rule. For example:

var rules = {
    'rules': {
        'number-leading-zero': [ 2, 'never' , 'blah blah blah'],
    }
};

Output: blah blah blah (number-leading-zero)

Where as of the default (no override), would be the normal:

var rules = {
    'rules': {
        'number-leading-zero': [ 2, 'never'],
    }
};

Output: Unexpected leading zero (number-leading-zero)

Is this something that can easily be done?

@bluetidepro bluetidepro changed the title Customize rule message text? Customize/override rule message text? Oct 1, 2015
@jeddy3
Copy link
Member

jeddy3 commented Oct 2, 2015

Hi. @davidtheclark is in a better position to comment on how easily this can be done. But before he does, I'm curious why you'd want this functionality. Can you tell us a little more about it please?

For example, if you have suggestions on how the messages could be better written, then by all means share those here, so that we can incorporate any improvements into the texts for the benefit or all the users. But if you've got a more specific use case, it'd be great to hear more about it.

@jeddy3 jeddy3 added status: needs discussion triage needs further discussion type: enhancement labels Oct 2, 2015
@davidtheclark
Copy link
Contributor

I think it would be straightforward to implement, but annoying because we'd have to change some code in all the many many many rules. But like @jeddy3, I'm curious about the use-case, because I don't think this is a typical feature of a linter and the desire isn't self-evident to me.

@bluetidepro
Copy link
Author

@jeddy3 @davidtheclark So I think it stems from a few use-cases...

  • If it was just a string you could override, then you could do things like add links to the description. This especially useful for our developers that may not be familiar with css, so it would help to link them to the stylint docs for the rule so they could see what the issue was.
  • It would allow people to add "voice" to the messages. So you could be like "hey, this is the rule that you broke, here is an example of how it should look, please fix it" or something like that. It would make it easier to be able to customize that level of the feedback.
  • It would be an easy way to get multi-language support in the messages. You could make custom message strings with the translated rule or something telling people what went wrong.

Those are just a few I could think of, but I think it would be super valuable in general. I'd be curious if you really did have to edit each rule to do this? You could possible just put it at the level where it outputs the message, and if there is a custom value in the rule set then overwrite the one in the rule? Just an idea.

More or less, I think it would be extremely useful in many ways to be able to modify that rule message.

@davidtheclark
Copy link
Contributor

@bluetidepro : Sound like good reasons, thanks! And you're right .... there might be some means of handling this behind the scenes without having to tweak each rule a little. So my take is it's well worth looking into but I won't be able to get to it myself for a little while.

@bluetidepro
Copy link
Author

@davidtheclark That's fair. I can look into it and then possible add a PR if I can easily figure it out. I'll see what I can come up with! :) Thanks, David!

@davidtheclark
Copy link
Contributor

I would dive in but am going backpacking tomorrow :)

@jeddy3
Copy link
Member

jeddy3 commented Oct 2, 2015

@bluetidepro Thanks for the explanation! It does sound like a worthwhile addition. Feel free to have a dig around in David's absence.

@davidtheclark Have a fab holiday! :) I'll catch up with you after the 11th.

@jeddy3 jeddy3 added status: pr welcome and removed status: needs discussion triage needs further discussion labels Oct 2, 2015
@davidtheclark
Copy link
Contributor

One problem here is that many of the messages are functions, something like maxLength =>You can't have a line longer than ${maxLength}`. But configuration needs to be JSON-friendly.

We could figure something out (maybe interpolation with indexed placeholders? e.g. 'No lines longer than {0}') But what this would mean is that every message would have to considered uniquely: each one may or may not pass any number of arguments. Which means for each rule we would have to somehow document how you can customize the message -- which is a big pain. Any solutions?

@jeddy3
Copy link
Member

jeddy3 commented Nov 2, 2015

One problem here is that many of the rules are functions

Oh yeah, good point. That does significantly complicate this feature.

Which means for each rule we would have to somehow document how you can customize the message -- which is a big pain. Any solutions?

None from me I'm afraid. Perhaps @bluetidepro might have some ideas?

I had one thought though. Perhaps it isn't an appropriate use of a formatter, but could the first two use cases be achieved using custom reporters? i.e. read the rule name from the message and then append either a link to the documentation or a friendly extra description with examples?

@davidtheclark
Copy link
Contributor

Yes, a custom formatter could do any of the above. But it's not very user-friendly, of course.

@davidtheclark davidtheclark added the status: ready to implement is ready to be worked on by someone label Nov 16, 2015
@jeddy3 jeddy3 changed the title Customize/override rule message text? Add ability to customize/override rule message text Jan 9, 2016
@kangax
Copy link
Contributor

kangax commented Jan 29, 2016

Wouldn't it make sense to start with custom messages that don't support placeholders? Our reasons are all the same as @bluetidepro mentioned earlier. With hundreds of people working on CSS, we have to explain to people what the rule is about, where to read more about it, and who to consult. So a plain message is usually enough in our case (it's just much more detailed; compare to current "no-important", for example — "Unexpected !important (declaration-no-important)" :))

@jeddy3
Copy link
Member

jeddy3 commented Jan 29, 2016

Wouldn't it make sense to start with custom messages that don't support placeholders?

Definitively :)

explain to people what the rule is about, where to read more about it, and who to consult

For this use case it sounds like a (private) custom formatter is probably the easiest and quickest way to get this done. You could then write extensive documentation for each rule without cluttering up your configuration file. You can also choose—on a per rule basis—whether to keep and append to the original message (with its string interpolation), or craft a new message (without any string interpolation) from scratch i.e. Unexpected !important (declaration-no-important) is not so useful, but Expected "#FFF" to be "#fff" (colour-hex-case) might be worth keeping around.

Is this or overwriting the messages in the config (without string interpolation for now) the best way to go?

@davidtheclark
Copy link
Contributor

We could also consider creating a new feature that is just for applying custom messages --- the idea being that people could use the regular formatter but only modify messages in a straightforward way. Like, user could provide an object mapping rule names to special messages (the message could even be a function!).

Let me flesh this idea out a little ... We add an option to all the APIs (plugin, standalone, CLI) for customMessages. It's an object that looks like this:

var myCustomMessages = {
  'block-no-empty': 'Why would you want an empty block? Are you making a mistake?',
  'max-line-length': max => `We are restricting our line length to ${max} characters ` +
    `so that we can all read code in split pane editors.`
};

When a rule complains, it looks to see if there's a special message for it; if not, it uses the standard message.

One limitation of this would be that you'd specify one message per rule. It assumes that you are customizing your message to your configuration, and if you change the config you'll have to remember change the message.

Also, I like @jeddy3's idea of being able to append to the original message ... would like to think of a way of working that in.

What do you think?

@bluetidepro
Copy link
Author

For what it's worth, here is what I did for our (Sprout Social, where I work) use case to make our own custom messages. It's not the cleanest per se, but it's an option. We use a custom reporter in the process to have custom message outputs: https://github.com/postcss/postcss-reporter

Here is a snippets with comments in the code on what's sort of going on with it:

var chalk = require('chalk');
var symbols = require('log-symbols');
var _ = require('lodash');
var stylelint = require('stylelint');
var reporter = require('postcss-reporter');

// Run postcss and stylelint
var stylelintPostcss = postcss([
    stylelint(),
    reporter({
        formatter: function(input) {
            // Custom formatter to display the warnings/errors
            var messages = input.messages;
            var output = '';

            // If there is a source, output it
            if (input.source) {
                output += chalk.underline(reporterLogFrom(input.source) + '\n');
            }

            // For each message (with content) loop the function `reporterMessagesFormat`, so lets customize how that output looks
            messages.forEach(function(message) {
                output += reporterMessagesFormat(message, input.source) + '\n';
            })

            // Ignore messages that are empty
            if(!_.isEmpty(messages)){
                return output;
            }
        },
        throwError: false
    })
]);

// Used in reporterMessagesFormat as a regex
var oldCodeRegexTest = /old\/components\/(.+)/;

// Ran for each message (with content)
function reporterMessagesFormat(message, source) {
    // This is a regex test we do so that certain .scss/css in certain folders is considered
    // an error while old code we haven't migrated yet is just a warning
    var isOldCode = oldCodeRegexTest.test(source);
    var str = '';
    if (message.text) {
        // Just get the message text
        var regExp = /(.+)( \(.+\))/; 
        var messageText = regExp.exec(message.text);
        // Get the column/line info
        if (message.line && message.column) {
            str += chalk.gray(message.line + ':' + message.column + ' ');
        }
        // Check whether we should call this an error or not
        if (message.severity === 'warning' && !isOldCode) {
            str += chalk.yellow(symbols.warning + ' Warning:');
        } else if(message.severity === 'error' || isOldCode) {
            str += chalk.red(symbols.error + ' Error:');
        }
        // Link to the rule so in apps like iTerm they can cmd click the link to open
        // to that rule in the browser
        str += ' ' + messageText[1];
        if (message.rule) {
            str += chalk.gray(' (' + chalk.blue('http://tiny.cc/pxoy7x/' + message.rule) + ')');
        }
    }
    return str;
}

One of my favorite parts of it is how it generated something like (http://tiny.cc/pxoy7x/no-eol-whitespace) which is a nice way to have a short link to the stylelint repo docs right to that rule. It's a neat little hack I did. And as you can see in the comment, in some terminal's like iTerm, you can CMD click the link to open your browser right to that rule!

And the output looks like (I added some fun little stuff to the output to make it look nice too haha):

stylelintPostcss

So you could easily do something like this to see which rule it is (message.rule in my code above), and then output a custom message.

Let me know if you have any specific questions or what to see more of the code of the full task! Hope this helps @kangax @jeddy3 @davidtheclark :)

@davidtheclark
Copy link
Contributor

@bluetidepro I love it! Very good to know that the custom formatter API worked out as planned to enable something like that.

@jeddy3
Copy link
Member

jeddy3 commented Jan 29, 2016

@davidtheclark said: We could also consider creating a new feature that is just for applying custom messages

Oh, I like this!

@bluetidepro It's fantastic to see your task and the output! Up until now we've not really had the chance to see how anyone is using stylelint in their own unique way - it's awesome to see :)

Like @davidtheclark said, it's also great to see the custom formatter API put to great use.

@bluetidepro said: a nice way to have a short link to the stylelint repo docs right to that rule

This is so cool :) It's also something I was thinking of adding to a custom formatter too!

@bluetidepro said: or want to see more of the code of the full task

That's kind of you. If it's not too much trouble are you able to paste into #659 the bit of the code where you tally up the total number of errors and warnings, as @s10wen was requesting exactly that feature just a couple of days ago? :)


So, it looks like using a custom formatter is very much capable of fulfilling @kangax needs. Although, I think @davidtheclark's proposal compliments the custom formatter API and offers a neater solution. @kangax What are you thoughts?

@bluetidepro
Copy link
Author

@jeddy3 No problem, I posted the full source of the grunt task here #659 (comment)

@kangax
Copy link
Contributor

kangax commented Jan 29, 2016

Thanks for your thoughts, folks!

The formatter is definitely the most flexible option for us. However, I also love the simplicity of solution proposed in this ticket (custom rule messages, either together with rule or in a standalone object).

The thing about custom formatter is that I'll still end up writing a standalone object that maps rule names to custom messages. In your case, @bluetidepro, you were able to do a clever rule explanation via an automatically generated url (in formatter) by convention.

In our case, it's probably not worth creating multiple pages for all the rules, so a simple map would be best. And considering that we're pretty satisfied with an existing formatter, we could totally do with just adding custom messages to existing rules.

Should the messages be in a separate object? I'm leaning towards "not". Things in different places usually get out of sync quick and are harder to maintain.

davidtheclark added a commit that referenced this issue Feb 1, 2016
Allow for custom messages; closes #400
@davidtheclark
Copy link
Contributor

@jeddy3
Copy link
Member

jeddy3 commented Feb 1, 2016

🎉

@AndyOGo
Copy link
Contributor

AndyOGo commented Feb 25, 2021

@jeddy3
@davidtheclark
I handle a custom message option with substitutions in my plugin.

Seem that it is ignored because of this general solution, so I deleted the customMessages entry for my rule, like:

delete result.stylelint.customMessages[ruleName];

An official way for a plugin to say, hey I'm handling custom messages by myself with support for interpolation would be great.

That is my issue: AndyOGo/stylelint-declaration-strict-value#142

@jeddy3
Copy link
Member

jeddy3 commented Feb 25, 2021

An official way for a plugin to say, hey I'm handling custom messages by myself with support for interpolation would be great.

Can you create a new feature request issue for this, please? Otherwise, it'll get lost as a comment here.

@AndyOGo
Copy link
Contributor

AndyOGo commented Feb 25, 2021

Done.
#5170

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: ready to implement is ready to be worked on by someone
Development

No branches or pull requests

5 participants