retextjs / retext-readability

plugin to check readability
https://unifiedjs.com
MIT License
94 stars 8 forks source link

Reword confidence messages and add ruleId #5

Closed localjo closed 8 years ago

localjo commented 8 years ago

Resolves #4. Rewords confidence messages and adds ruleId. I decided to use the word "confidence" instead of "sureness" both in the messages and internally for clarity and consistency. Messages now look like this;

5:1-5:68  warning    Hard to read sentence, low confidence           hard-to-read
6:1-6:86  warning    Hard to read sentence, moderate confidence      hard-to-read
7:1-7:98  warning    Hard to read sentence, high confidence          hard-to-read
localjo commented 8 years ago

Oops, I need to fix tests and update the README. :)

codecov-io commented 8 years ago

Current coverage is 100% (diff: 100%)

Merging #5 into master will not change coverage

@@           master    #5   diff @@
===================================
  Files           2     2          
  Lines         139   127    -12   
  Methods        18    19     +1   
  Messages        0     0          
  Branches       17    14     -3   
===================================
- Hits          139   127    -12   
  Misses          0     0          
  Partials        0     0          

Powered by Codecov. Last update 984bcd6...d960bae

localjo commented 8 years ago

Does this look good to merge now?

wooorm commented 8 years ago

Sorry for the mixed signals, but two more Qs 😐

  1. How about confidenceLabel instead of confidenceLevel, as a property? It’s just a bit confusing looking at those properties what they mean, confidenceLabel clearly implies it’s a label, so they other one must be the actual raw value... Thoughts?
  2. I slightly don’t like the wording of the lowest tier confidence. “Low” sounds a bit too... low?! So, instead of high, moderate, low; maybe very high, high, moderate?
wooorm commented 8 years ago

Or, certain, likely, probable? https://en.wikipedia.org/wiki/Words_of_estimative_probability

localjo commented 8 years ago

I think certain, likely, probable still sound a little ambiguous. very high, high, moderate seem clearer. Or we could just do away with the label altogether, and calculate the message description as a percentage like this;

'Hard to read sentence (confidence: ' + (confidence*100).toFixed() + '%)'
// Hard to read sentence (confidence: 71%)
wooorm commented 8 years ago

As those numbers are a bit random, how about Hard to read sentence (confidence: 5/7) and the like?

localjo commented 8 years ago

Displaying the numbers as fractions might be a little bit more difficult, since JS is doing the division automatically. I think displaying them as percentages is intuitive. Right now, four percentages could be displayed, 100% (7/7), 86% (6/7), 71% (5/7), and 57% (4/7). If the user passes a lower threshold option in, lower percentages could be displayed as well. The advantage of using the percentage is that it gives people an intuitive idea of the meaning without having to understand and remember that there are 7 different formulas being used. And if another formula is added (or removed) in the future, the code can dynamically adapt to that.

wooorm commented 8 years ago

@josiahsprague Well, we count the succeeding algorithms right? Just before confidence /= length, confidence is number 0..7! Something like this:

'Hard to read sentence (confidence: ' + success + '/7)'

...where succes is confidence just before confidence is modified?

wooorm commented 8 years ago

Tip: use 'npm run test' locally, to check if code style is OK!

localjo commented 8 years ago

Just noticed that. :)

localjo commented 8 years ago

So I refactored some of the code to make it simpler. Since we're just labeling with the confidence string now, I removed the constants for the separate levels. I also changed the code used to get the count of failed formulas to use Array.prototype.filter() so that it's more concise. And I'm just adding message.confidence to the message object as a string that looks like this; '5/7'.

How's that look?

wooorm commented 8 years ago

Done! Thanks so much, @josiahsprague, lovely to have had your help!

I did revert the filter to a while loop, though, I really dig those :)

localjo commented 8 years ago

Sounds good. :) Will you be releasing updates to npm soon?

wooorm commented 8 years ago

Yup, in a sec

wooorm commented 8 years ago

Released!