Consider using :command in checker definition

[nnicandro]

Hello,

Thanks for writing this 👍.

I see that you have updated your code from the version of a couple of days ago. You are very busy on this!

But I was wondering if you would consider using flycheck-define-command-checker which would essentially do all of the process handling that you are working in your most recent commits. I checked the flycheck version you are supporting, 0.22, and it seems that this command is available.

Using flycheck-define-command-checker I was able to get a working asynchronous checker with the following:

(defcustom flycheck-vale-modes '(text-mode markdown-mode rst-mode)
  "List of major modes in which to apply this checker."
  :type '(repeat function))

(defconst flycheck-vale--level-map
  '(("error" . error)
    ("warning" . warning)))

(defun flycheck-vale--output-to-errors (output checker buffer)
  "Parse the full JSON output of vale, OUTPUT, into a sequence of flycheck error structs."
  (let* ((full-results (json-read-from-string output))
         (filename (buffer-file-name buffer))
         ;; Get the list of issues for each file.
         (result-vecs (mapcar 'cdr full-results))

         ;; Chain all of the issues together. The point here, really, is that we
         ;; don't expect results from more than one file, but we should be
         ;; prepared for the theoretical possibility that the issues are somehow
         ;; split across multiple files. This is basically a punt in lieu of
         ;; more information.
         (issues (apply 'concatenate 'list (mapcar 'cdr full-results))))
    (mapcar (lambda (issue)
         (let-alist issue
           (flycheck-error-new
            :buffer buffer
            :filename filename
            :checker checker
            :line .Line
            :column (elt .Span 0)
            :message .Message
            :level (assoc-default .Severity flycheck-vale--level-map 'string-equal 'error))))
       issues)))

;;;###autoload
(defun flycheck-vale-setup ()
  "Convenience function to setup the vale flycheck checker.

This adds the vale checker to the list of flycheck checkers."
  (add-to-list 'flycheck-checkers 'vale))

(flycheck-def-executable-var vale "vale")

(flycheck-define-command-checker 'vale
  "A flycheck checker using vale natural language linting."
  :command '("vale" "--output" "JSON" source)
  :error-parser #'flycheck-vale--output-to-errors
  :modes flycheck-vale-modes)

Note that flycheck-def-executable-var would define a custom variable, flycheck-vale-executable which replaces flycheck-vale-program.

[abingham]

That looks great! To create my version I just copied another checker that I found, and your approach seems much more idiomatic. Would you mind putting together a PR? I'll give it a try and merge it in as quickly as I can. Thanks!

[nnicandro]

I found out about flycheck-define-command-checker and flycheck-def-executable-var by browsing the implementation of the checkers in flycheck 😃. Once this checker is ready it probably wouldn't be a bad idea to submit a pull request over there since I don't think they have any prose linters.

[abingham]

submit a pull request over there

Do you mean move this project into the flycheck project on github? I'd consider that if they'll take responsibility for maintaining it. Otherwise, what's the benefit to doing so? (I'm genuinely asking...it's not clear to my why that move would benefit them, me, or the project).

[nnicandro]

flycheck does not have these kinds of style based checkers and seems to be on the verge of supporting them since there is a pull request for proselint, a linter similar to vale (https://github.com/flycheck/flycheck/pull/939). I found proselint to be too slow compared to vale when working with large org-mode files and so it would be beneficial to have vale as another option once they start supporting style based checkers (https://github.com/flycheck/flycheck/pull/961).

[abingham]

Is the main benefit of being part of flycheck visibility and availability? I don't see any technical advantage to integrating directly into it; it's fairly straightforward to use flycheck-vale as it is, I think.

With that said, there's a lot to be said for being visible and available. I guess I'm not personally motivated enough to make a PR to flycheck, but I'd be happy to support one if you or someone wants to put it together. My primary motivation is just to have access to vale via flycheck.

[abingham]

(I'm closing this since we've merged in your PR, but we can continue talking about flycheck integration here if you want).