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.

Sign up for GitHub

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

[WiP] Added a prog mode to flycheck-languagetool #25

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+52 −4
Open

[WiP] Added a prog mode to flycheck-languagetool #25

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 52 additions & 4 deletions flycheck-languagetool.el
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,9 @@ or plan to start a local server some other way."
These rules will be disabled if Emacs’ `flyspell-mode' or
`jinx-mode' is active.")

(defconst flycheck-languagetool--prog-rules
'("COMMA_PARENTHESIS_WHITESPACE"))

;;
;; (@* "External" )
;;
Expand All @@ -161,6 +164,47 @@ These rules will be disabled if Emacs’ `flyspell-mode' or
(unless pt (setq pt (point)))
(save-excursion (goto-char pt) (current-column)))

;;
;; (@* "Prog Mode" )
;;

(defvar flycheck-languagetool--text-mode t)

(defcustom flycheck-languagetool-prog-text-faces
(if (bound-and-true-p tree-sitter-hl-mode)
'(tree-sitter-hl-face:comment tree-sitter-hl-face:doc tree-sitter-hl-face:string)
'(font-lock-string-face font-lock-comment-face font-lock-doc-face))
"Faces corresponding to text in programming-mode buffers."
:type '(repeat (const font-lock-string-face)))

(defun flycheck-languagetool--generic-progmode-verify (pt-beg pt-end)
"Used for `flycheck-languagetool-generic-check-word-predicate'.
This predicate checks that the word between PT-BEG and PT-END has
a \"text\" face in programming modes."
(unless (eql pt-beg pt-end)
;; (point) is next char after the word. Must check one char before.
(let ((f (get-text-property (1- pt-end) 'face)))
(memq f flycheck-languagetool-prog-text-faces))))

(defun flycheck-languagetool-flycheck-add-mode (mode)
"Register flycheck languagetool support for MODE."
(unless (flycheck-checker-supports-major-mode-p 'languagetool mode)
(flycheck-add-mode 'languagetool mode)))

;;;###autoload
(defun flycheck-languagetool-flycheck-enable ()
"Enable flycheck languagetool integration for the current buffer.
This adds languagetool as the next checker of the main checker
of the current buffer"
(interactive)
(require 'flycheck)
(flycheck-mode 1)
(flycheck-stop)
(flycheck-languagetool-flycheck-add-mode major-mode)
(add-to-list 'flycheck-checkers 'languagetool)
(setq flycheck-languagetool--text-mode nil)
(flycheck-add-next-checker (flycheck-get-checker-for-buffer) 'languagetool))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few comments:

  1. Maybe we should get rid of the (require 'flycheck-mode) here since we already have it at the very top.
  2. I don't think enabling flycheck-mode and such a setup is a good idea, we usually let the user decide weather the flycheck-mode is enabled or not. Of course, we can still expose a function for that but we need to clarify so the user don't get confuse between flycheck-languagetool-flycheck-enable and flycheck-languagetool-setup. (Might need to update the README for this)
  3. Should the variable flycheck-languagetool--text-mode be defvar-local? Anyway, I think it's better if we reuse flycheck-languagetool-setup for prog-mode as well. For example,
(defcustom flycheck-languagetool-prog-modes
  '(actionscript-mode haxe-mode nxml-mode yaml-mode)
  "List of extra `prog-mode'.")

(defun flycheck-languagetool-prog-mode-p ()
  "Return non-nil if current buffer is programming mode."
  (or (derived-mode-p 'prog-mode)
      (memq major-mode flycheck-languagetool-prog-modes)))

then

(defun flycheck-languagetool-setup ()
  "Setup flycheck-package."
  (interactive)
  (when (flycheck-languagetool-prog-mode-p)
    (flycheck-languagetool-flycheck-add-mode major-mode)
    (flycheck-add-next-checker (flycheck-get-checker-for-buffer) 'languagetool))
  ;; do it whenever this is programming mode or not
  (add-to-list 'flycheck-checkers 'languagetool))

;;;###autoload
(defun flycheck-languagetool-flycheck-enable ()
  "Enable flycheck languagetool integration for the current buffer.
This adds languagetool as the next checker of the main checker
of the current buffer"
  (interactive)
  (flycheck-mode 1)
  (flycheck-stop)
  (setq flycheck-languagetool--text-mode nil)  ; still not quite sure what this does
  (flycheck-languagetool-setup))  ; reuse it

This way, user wouldn't need to do extra configuration for prog-mode support. BTW, above code is an example, so please change it to make it work! ;)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review! This is clearly a work in progress, I needed to do something minimal that worked to start the process of iterating over it. I'll look into your suggestions :-)


;;
;; (@* "Core" )
;;
Expand All @@ -182,10 +226,11 @@ These rules will be disabled if Emacs’ `flyspell-mode' or
(desc (cdr (assoc 'message match)))
(col-start (flycheck-languagetool--column-at-pos pt-beg))
(col-end (flycheck-languagetool--column-at-pos pt-end)))
(push (list ln col-start type desc
:end-column col-end
:id (cons id subid))
check-list)))
(when (or flycheck-languagetool--text-mode (flycheck-languagetool--generic-progmode-verify pt-beg pt-end))
(push (list ln col-start type desc
:end-column col-end
:id (cons id subid))
check-list))))
check-list))

(defun flycheck-languagetool--read-results (status source-buffer callback)
Expand Down Expand Up @@ -255,6 +300,8 @@ CALLBACK is passed from Flycheck."
(flatten-tree (list
(cdr (assoc "disabledRules"
flycheck-languagetool-check-params))
(unless flycheck-languagetool--text-mode
flycheck-languagetool--prog-rules)
(when (or (bound-and-true-p flyspell-mode)
(bound-and-true-p jinx-mode))
flycheck-languagetool--spelling-rules))))
Expand Down Expand Up @@ -370,6 +417,7 @@ CALLBACK is passed from Flycheck."
(defun flycheck-languagetool-setup ()
"Setup flycheck-package."
(interactive)
(setq flycheck-languagetool--text-mode t)
(add-to-list 'flycheck-checkers 'languagetool))

(provide 'flycheck-languagetool)
Expand Down