emacs-grammarly / flycheck-grammarly

Grammarly support for Flycheck
GNU General Public License v3.0
127 stars 8 forks source link

Doesn't seem to work with non-trivial files? #3

Closed mpereira closed 3 years ago

mpereira commented 4 years ago

Hi,

Thanks for writing this package, it looks like it could be very useful for my use cases. I was unable to get it working with files that contain anything more than simple sentences. Maybe it's due to something in my environment?

With this configuration:

(setq debug-on-error t)

(add-hook 'after-init-hook
          (lambda ()
            (setq debug-on-error nil)))

(require 'package)

(setq package-enable-at-startup nil)

(package-initialize)

(setq package-archives '(("org" . "https://orgmode.org/elpa/")
                         ("gnu" . "https://elpa.gnu.org/packages/")
                         ("melpa" . "https://melpa.org/packages/")))

(unless package-archive-contents
  (package-refresh-contents))

(unless (package-installed-p 'use-package)
  (package-refresh-contents)
  (package-install 'use-package))

(eval-when-compile
  (require 'use-package))

(setq use-package-always-ensure t)
(setq use-package-always-demand t)

;; macOS modifiers.
(setq mac-command-modifier 'meta)
;; Setting "Option" to nil allows me to type umlauts with "Option+u".
(setq mac-option-modifier nil)
(setq mac-control-modifier 'control)
(setq ns-function-modifier 'hyper)

(use-package flycheck)
(use-package flycheck-grammarly)

and this text (from https://orgmode.org/worg/org-tutorials/org4beginners.html, added first paragraph)

** Lord of the Rings
   My name Bob.

   My favorite scenes are (in this order)
   1. The attack of the Rohirrim
   2. Eowyn's fight with the witch king
      + this was already my favorite scene in the book
      + I really like Miranda Otto.
   3. Peter Jackson being shot by Legolas
       - on DVD only
      He makes a really funny face when it happens.
   But in the end, no individual scenes matter but the film as a whole.
   Important actors in this film are:
   - Elijah Wood :: He plays Frodo
   - Sean Austin :: He plays Sam, Frodo's friend.  I still remember
     him very well from his role as Mikey Walsh in The Goonies.

enabling flycheck-mode doesn't do anything. Changing the text to

** Lord of the Rings
   My name Bob.

correctly shows the flycheck error: "Your sentence appears to be missing a verb."

I have also tried exporting the document to Markdown and the behavior is the same.

Please let me know if you need more information.

GNU Emacs 28.0.50 (build 1, x86_64-apple-darwin19.5.0, NS appkit-1894.50 Version 10.15.5 (Build 19F101)) of 2020-10-03

flycheck-grammarly.el

;; Version: 0.1.5
;; Package-Version: 20200823.926
;; Package-Commit: 4ee6480871df74b05ada5f0afab5190930a70be9
jcs090218 commented 4 years ago

Interesting... This actually gives me a score of 100, which is very weird. I think is an issue from parsing layer but I am not confident with it. Let me investigate a few days and I will get back to you ASAP!

mpereira commented 4 years ago

Thanks for looking into it!

ghost commented 3 years ago

I can also confirm this behavior. The behavior is broken for larger files. And it does indeed return a score of 100.

jcs090218 commented 3 years ago

Sorry for the late reply!

I am currently busy with something else and not having sufficient of time to contribute to this project. I will try to resolve this issue as soon as I have time to do it! Sorry for the inconvenience! 😖

mpereira commented 3 years ago

No worries, and no pressure! We all have lives to live. 🙂

rudolf-adamkovic commented 3 years ago

I investigated this issue and found that, if an Org document contains at least one colon character (":"), flycheck-grammarly stops working. If I replace all colons with nothing (""), flycheck-grammarly works again, even with the most complex documents.

P.S.flymake-grammarly has this bug too.

jcs090218 commented 3 years ago

I investigated this issue and found that, if an Org document contains at least one colon character (":"), flycheck-grammarly stops working. If I replace all colons with nothing (""), flycheck-grammarly works again, even with the most complex documents.

@salutis If this is true, I can apply quick patch by replacing all : to something else (maybe a comma ,). Do you think this will work? Thanks for your hard work by further investigate this issue! 😖 👍

@mpereira @PreciousPudding Can someone confirm this action?

rudolf-adamkovic commented 3 years ago

@salutis If this is true, I can apply quick patch by replacing all : to something else (maybe a comma ,). Do you think this will work?

I think so. My workflow now is to replace all colons locally to a temporary string, then fix my grammar, and then replace the temporary strings back to colons. It would be fabulous if flycheck-grammarly could do that for me.

As for using a comma, I am not sure. For instance, the sentence

The colors of buses are: red, green, and blue.

is correct, but the sentence

The colors of buses are, red, green, and blue.

raises a warning in Grammarly.

I would suggest to replace all colons with some other Unicode colon. (Or better, investigate what is happening, if the error is on our side.) I tried two colons from this list and Grammarly did not raise a warning:

https://unicode-search.net/unicode-namesearch.pl?term=COLON

jcs090218 commented 3 years ago

raises a warning in Grammarly.

Yeah, that's a rash decision. Should probably replace with something else.

The solution should be simple, by wrapping (buffer-sting) should work 😕

https://github.com/emacs-grammarly/flycheck-grammarly/blob/8321fc98a0809cad17e37ca924d364423c37b8c0/flycheck-grammarly.el#L192

The only thing we need to concern is what character should use to replace colons : and the character must be the length of 1, so we can keep the result the same without needing to re-calculate the error point. @salutis Would you mind doing some test for this? 😕

Anyway, feel free to open a PR if you already have the fix! 👍

jcs090218 commented 3 years ago

This issue should be fixed in 5568feaa5e71cf58e4d89a5aa92e2242250bd02c.

rudolf-adamkovic commented 3 years ago

This issue should be fixed in 5568fea.

Thank you so much! ❤️