Add on-the-fly easy score checking #27

ag91 · 2018-12-04T20:37:40Z

Closes #25

Looking forward to get some feedback about this :)

ag91 · 2018-12-04T20:44:11Z

I have added also a function that toggles the on-the-fly mode. By default is turned off.

By the way I am not totally sure I autoload all the required functions, so a review on that would be idea.

bnbeckwith · 2018-12-04T20:46:03Z

Let me quickly say "thank you" for the PR. I'll review it shortly.

Default off is a good idea. In fact, for a feature like this, it is a requirement in my book.

robstewart57 · 2018-12-05T11:39:10Z

I agree that the default should be this feature being turned off.

@ag91 it'd be good to include documentation, both in README.org and in writegood-mode.el, stating how to enable/disable this feature.

ag91 · 2018-12-05T15:27:34Z

@robstewart57 that makes perfect sense! Too functional oriented XD I will update with that change

ag91 · 2018-12-05T22:15:57Z

Hi, updated the README. Please review the style: I am not sure it fits with the tone of the original text. Also while writing I was thinking if it would make sense to make the hooks customizable. For example, somebody may want to check a sentence score even after a semicolon, rather than just a dot. What do you think?

…

On Wed 05 Dec 2018 at 12:39, Rob Stewart ***@***.***> wrote: I agree that the default should be this feature being turned off. @ag91 it'd be good to include documentation, both in README.org and in writegood-mode.el, stating how to enable/disable this feature. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

bnbeckwith

@ag91 Thank you for this PR. It works and I like the idea behind the feature addition.

Before this gets merged, there are some changes to make the solution more robust and better integrate into emacs. If any of my comments are confusing, feel free to fire back. I'd like to make sure we have the conversations that we need to ship this feature.

bnbeckwith · 2018-12-04T20:46:37Z

writegood-mode.el

-;;;###autoload
-(defun writegood-reading-ease (&optional start end)
-  "Flesch-Kincaid reading ease test in the region bounded by START and END.
+;;; BEGIN Score on the fly


What does BEGIN mean to you here?

I will remove this: it was only to delimit my additions while I was merging stuff in.

bnbeckwith · 2018-12-04T20:47:21Z

writegood-mode.el

   (let* ((params (writegood-fk-parameters start end))
          (sentences (nth 0 params))
          (words     (nth 1 params))
          (syllables (nth 2 params))
          (score  (- 206.835 (* 1.015 (/ words sentences)) (* 84.6 (/ syllables words)))))
+     score))


Do we need so store score via let if we are just returning it? Should we just return the calculation instead?

Not really, I think I was doing some debugging with prints at the time. I will update that as well.

bnbeckwith · 2018-12-06T21:04:16Z

writegood-mode.el

@@ -152,6 +152,12 @@
  :group 'writegood
  :type '(repeat character))

+(defcustom writegood-on-the-fly-modes


I'm no sure I understand why we would want to restrict the modes here?

If I turn on writegood in any mode and then toggle on the on-the-fly feature, it should work. Is there a reason to add this filtering/checking step?

So far this mode acts globally when you toggle it. This means that if you are programming, writegood-mode would try to calculate the score of your code (which does not seem to make sense). For this reason I restricted to only those modes I found this feature useful.

A sensible alternative would be to run the feature only for the current buffer.

I just like it activated by default in multiple modes because I do not need to think about activating useful stuff while I am focusing on writing. Sometimes I just think: "this sentence does not look good" and then I find confirmation in the sentence score in the modeline :)

bnbeckwith · 2018-12-06T21:17:08Z

writegood-mode.el

+
+(defun writegood-on-the-fly-turn-on ()
+  "Add hooks to enable on-the-fly scoring."
+  (add-hook 'post-command-hook 'writegood-after-sentence-hook)


I think we can probably use something like this instead:

(add-hook 'post-self-insert-hook 'syntax-check-continuous nil t)

Really, the post-self-insert-hook is the part we want. That will take away the need to string-match the command name. We'll still need to check for a sentence end.

This sounds interesting! I will play around with it and propose an alternative.

bnbeckwith · 2018-12-06T21:18:07Z

writegood-mode.el

+(defun writegood-after-sentence ()
+  "Calculate reading ease after a sentence is completed. Here we
+consider the sentence completion the addition of a dot."
+  (if (and (string-match-p (regexp-quote "self-insert-command") (symbol-name last-command)) (eq (char-before) 46)) (writegoodmode-reading-ease-sentence)))


Is char 46 a '.'? What if a sentence ends with a question-mark? Or exclamation point!? We'll need a more general check for sentence end to apply here.

You are perfectly right! I did not think about it at all.
Should I propose a set of characters that delimit a sentence or you have already a list in mind?
Maybe I can find something already done for thing-at-point 'sentence

bnbeckwith · 2018-12-06T21:28:14Z

writegood-mode.el

+(defun writegood-on-the-fly-turn-on ()
+  "Add hooks to enable on-the-fly scoring."
+  (add-hook 'post-command-hook 'writegood-after-sentence-hook)
+  (add-hook 'post-command-hook 'writegood-after-paragraph-hook)


I wonder if we can use fill-paragraph-function here to avoid checking the last command and better plug into the fill architecture.

That is a very cool comment: I will both improve the code and learn more about Emacs fill architecture :D
I will give a look into this, and propose an alternative.

ag91 · 2018-12-07T11:38:19Z

Thanks for the review/comments! It sounds I will learn quite a bit by acting on them.
Very welcomed!

ag91 · 2018-12-17T17:01:45Z

@bnbeckwith how is it going? I think I can work on it a bit more after Christmas if you have further feedback

ag91 force-pushed the master branch from 7d62b7e to 1d2b76f Compare December 5, 2018 22:12

bnbeckwith requested changes Dec 6, 2018

View reviewed changes

Add on-the-fly easy score checking and improve README

575ad46

ag91 force-pushed the master branch from 1d2b76f to 575ad46 Compare December 9, 2018 18:39

tpeacock19 mentioned this pull request Apr 5, 2021

Missing Writegood-mode Functions ag91/writer-word-goals#2

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add on-the-fly easy score checking #27

Add on-the-fly easy score checking #27

ag91 commented Dec 4, 2018

ag91 commented Dec 4, 2018

bnbeckwith commented Dec 4, 2018

robstewart57 commented Dec 5, 2018

ag91 commented Dec 5, 2018

ag91 commented Dec 5, 2018 via email

bnbeckwith left a comment

bnbeckwith Dec 4, 2018

ag91 Dec 7, 2018

bnbeckwith Dec 4, 2018

ag91 Dec 7, 2018

bnbeckwith Dec 6, 2018

ag91 Dec 7, 2018

bnbeckwith Dec 6, 2018

ag91 Dec 7, 2018

bnbeckwith Dec 6, 2018

ag91 Dec 7, 2018

bnbeckwith Dec 6, 2018

ag91 Dec 7, 2018

ag91 commented Dec 7, 2018

ag91 commented Dec 17, 2018

Add on-the-fly easy score checking #27

Are you sure you want to change the base?

Add on-the-fly easy score checking #27

Conversation

ag91 commented Dec 4, 2018

ag91 commented Dec 4, 2018

bnbeckwith commented Dec 4, 2018

robstewart57 commented Dec 5, 2018

ag91 commented Dec 5, 2018

ag91 commented Dec 5, 2018 via email

bnbeckwith left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ag91 commented Dec 7, 2018

ag91 commented Dec 17, 2018