Flymake support

[minad]

Hi Jonas,

I just stumbled over flycheck-hl-todo, which seems quite useful. Would it make sense to add a corresponding flymake variant here, maybe in a separate file hl-todo-flymake.el or should this rather be a separate project? Thanks!

[tarsius]

I would prefer keeping it in a separate repository, but we should mention these packages in the readme.

[minad]

In the case of flycheck-hl-todo I agree that it should be separate, also because of the flycheck dependency. In the case of flymake however, it may be more economical to add the function here? It is pretty short, too short to justify a separate package. Note that a separate package flymake-hl-todo does not yet exist, perhaps this wasn't clear from above. What do you think?

;;;###autoload
(defun hl-todo-flymake (report-fn &rest _args)
  "Flymake backend, diagnostics are passed to REPORT-FN."
  (when hl-todo-mode
    (let (diags)
      (save-excursion
        (save-restriction
          (save-match-data
            (goto-char (point-min))
            (while (hl-todo--search)
              (let ((beg (match-beginning 0))
                    (end (pos-eol)))
                (push (flymake-make-diagnostic
                       (current-buffer) beg end :note
                       (buffer-substring-no-properties beg end))
                      diags))))))
      (when diags (funcall report-fn diags)))))

The user can add the function to flymake-diagnostic-functions if desired.

(add-hook 'hl-todo-mode-hook
  (lambda ()
    (add-hook 'flymake-diagnostic-functions #'hl-todo-flymake nil 'local)))

The flymake function could probably be implemented such that it is deferred and processes chunks in an idle timer. This would make it slightly more complex and may matter in large buffers. (EDIT: It seems flymake also provides change start and end positions, so these could also be used to make this more efficient.)

[minad]

Here is a more efficient variant which uses the :changes-start and :changes-end positions provided by Flymake. It needs a bit of testing but besides that is probably good to go.

;;;###autoload
(defun hl-todo-flymake (report-fn &rest plist)
  "Flymake backend for `hl-todo-mode'.
Diagnostics are reported to REPORT-FN and additional options are
given as PLIST."
  (let (diags rbeg rend)
    (when hl-todo-mode
      (save-excursion
        (save-restriction
          (save-match-data
            (goto-char (or (plist-get plist :changes-start) (point-min)))
            (setq rbeg (pos-bol))
            (goto-char (or (plist-get plist :changes-end) (point-max)))
            (setq rend (pos-eol))
            (goto-char rbeg)
            (while (hl-todo--search nil rend)
              (let ((beg (match-beginning 0))
                    (end (pos-eol)))
                (push (flymake-make-diagnostic
                       (current-buffer) beg end :note
                       (buffer-substring-no-properties beg end))
                      diags)))))))
    (apply report-fn (nreverse diags)
           (and rbeg `(:region (,rbeg . ,rend))))))

[tarsius]

It is pretty short, too short to justify a separate package.

If it stays contained in a single function, then feel free to finish it and open a pull-request. Please try to avoid loading flymake at runtime, just to avoid a compile time warning about undefined functions.

[minad]

If it stays contained in a single function, then feel free to finish it and open a pull-request. Please try to avoid loading flymake at runtime, just to avoid a compile time warning about undefined functions.

Thanks. Yes, the backend will stay contained in a single function and requiring flymake is also not necessary. I will test the backend a little more and then open a PR.

[minad]

Implemented in #74