New Flymake rewrite in emacs-26

New Flymake rewrite in emacs-26

[João Távora]

Hi,

As discussed, I have just merged the Flymake rewrite into emacs-26. I
did this sooner than I expected so people can give it as much testing as
possible.

`flymake-mode' is not active by default, so when writing Elisp, I
encourage you to try out the new backends. Just M-x flymake-mode or

  (add-to-list 'emacs-lisp-mode-hook 'flymake-mode).

Of course if you were already using Flymake for C, C++, etc. Everything
should also still work (slightly better, in fact). Let me know if it
doesn't.

Some things worth noting:

* There are two older commits in emacs-26 that must be merged to master
  after all, otherwise the merge-back of this feature won't work.

  This is because there was a reversion of the decision to bring this
  feature to emacs-26 (see
  https://lists.gnu.org/archive/html/emacs-devel/2017-09/msg01020.html
  and
  https://lists.gnu.org/archive/html/emacs-devel/2017-09/msg01030.html)
  the reversion

  The two commits are

  ce540f8a68..: João Távora 2017-09-27 Revert "Split flymake.el into flymake-proc.el and flymake-ui.el"
  7cf59c6635..: João Távora 2017-09-27 Revert "Add flymake-backends defcustom"

* The most important docstrings are in place, but the Texinfo
  documentation (which I also rewrote considerably) is missing the bits
  about the new API for writing backends.

  Ideally, I would like some feedback on it (besides Stefan, of
  course). Please see the docstrings of `flymake-diagnostic-functions'
  and `flymake-diagnostic-types-alist'.

  Even more ideally, someone would write a nicer backend for Emacs's C
  sources (the current "legacy" one requires Makefiles to contain a
  "syntax-check" target, which isn't practical, I presume)

Thanks,
João

Re: New Flymake rewrite in emacs-26

[Eli Zaretskii]

> From: joaotavora@gmail.com (João Távora)
> Date: Tue, 03 Oct 2017 15:05:44 +0100
> 
> As discussed, I have just merged the Flymake rewrite into emacs-26. I
> did this sooner than I expected so people can give it as much testing as
> possible.

Thanks.

> `flymake-mode' is not active by default, so when writing Elisp, I
> encourage you to try out the new backends. Just M-x flymake-mode or
> 
>   (add-to-list 'emacs-lisp-mode-hook 'flymake-mode).
> 
> Of course if you were already using Flymake for C, C++, etc. Everything
> should also still work (slightly better, in fact). Let me know if it
> doesn't.

This should be in NEWS, perhaps after adjusting the style a bit.

> * The most important docstrings are in place, but the Texinfo
>   documentation (which I also rewrote considerably) is missing the bits
>   about the new API for writing backends.

We should add some minimal documentation about the new features.

Re: New Flymake rewrite in emacs-26

[Lele Gaifax]

Minor doc fix:

    diff --git a/lisp/progmodes/flymake.el b/lisp/progmodes/flymake.el
    index 0b28dc31cf..1e959d884c 100644
    --- a/lisp/progmodes/flymake.el
    +++ b/lisp/progmodes/flymake.el
    @@ -335,7 +335,7 @@ flymake-diagnostic-functions
     * the remaining arguments are keyword-value pairs in the
       form (:KEY VALUE :KEY2 VALUE2...).  Currently, Flymake provides
       no such arguments, but backend functions must be prepared to
    -  accept to accept and possibly ignore any number of them.
    +  accept and possibly ignore any number of them.

     Backend functions are expected to initiate the buffer check, but
     aren't required to complete it check before exiting: if the

ciao, lele.
-- 
nickname: Lele Gaifax | Quando vivrò di quello che ho pensato ieri
real: Emanuele Gaifas | comincerò ad aver paura di chi mi copia.
lele@metapensiero.it  |                 -- Fortunato Depero, 1929.

Re: New Flymake rewrite in emacs-26

[Lele Gaifax]

Will make a proper bug report with a patch, if you prefer.

    diff --git a/doc/misc/flymake.texi b/doc/misc/flymake.texi
    index 5dd72f81e2..18d9d05e62 100644
    --- a/doc/misc/flymake.texi
    +++ b/doc/misc/flymake.texi
    @@ -182,7 +182,7 @@ Syntax check statuses
     @cindex Syntax check statuses

     After syntax check is finished, its status is displayed in the mode line.
    -The following statuses are defined.
    +The following statuses are defined:

     @multitable @columnfractions 0.25 0.75
     @item @code{Wait}
    @@ -356,7 +356,7 @@ Adding support for a new syntax check tool
     @code{init-function} is required to initialize the syntax check,
     usually by creating a temporary copy of the buffer contents.  The
     function must return @code{(list cmd-name arg-list)}.  If
    -@code{init-function} returns null, syntax check is aborted, by
    +@code{init-function} returns null, syntax check is aborted, but
     @code{flymake-mode} is not switched off.

     @item cleanup-function
    diff --git a/lisp/progmodes/flymake.el b/lisp/progmodes/flymake.el
    index 0b28dc31cf..1e959d884c 100644
    --- a/lisp/progmodes/flymake.el
    +++ b/lisp/progmodes/flymake.el
    @@ -335,7 +335,7 @@ flymake-diagnostic-functions
     * the remaining arguments are keyword-value pairs in the
       form (:KEY VALUE :KEY2 VALUE2...).  Currently, Flymake provides
       no such arguments, but backend functions must be prepared to
    -  accept to accept and possibly ignore any number of them.
    +  accept and possibly ignore any number of them.

     Backend functions are expected to initiate the buffer check, but
     aren't required to complete it check before exiting: if the

ciao, lele.
-- 
nickname: Lele Gaifax | Quando vivrò di quello che ho pensato ieri
real: Emanuele Gaifas | comincerò ad aver paura di chi mi copia.
lele@metapensiero.it  |                 -- Fortunato Depero, 1929.

Re: New Flymake rewrite in emacs-26

[João Távora]

[-- Attachment #1: Type: text/plain, Size: 2624 bytes --]

Hi, Lele

Thank you very much for proof-reading the doc. I make a lot of silly
mistakes.

I wonder if you've read the docstrings in the .el files, too, since that's
where most of the new documentation is right now.

> Will make a proper bug report with a patch, if you prefer:

Neither of those is essential I think. Unless someone disagrees, keep
posting to this thread and I'll make a commit crediting you as the author.

Thanks,
João



On Wed, Oct 4, 2017 at 2:41 PM, Lele Gaifax <lele@metapensiero.it> wrote:

> Will make a proper bug report with a patch, if you prefer.
>
>     diff --git a/doc/misc/flymake.texi b/doc/misc/flymake.texi
>     index 5dd72f81e2..18d9d05e62 100644
>     --- a/doc/misc/flymake.texi
>     +++ b/doc/misc/flymake.texi
>     @@ -182,7 +182,7 @@ Syntax check statuses
>      @cindex Syntax check statuses
>
>      After syntax check is finished, its status is displayed in the mode
> line.
>     -The following statuses are defined.
>     +The following statuses are defined:
>
>      @multitable @columnfractions 0.25 0.75
>      @item @code{Wait}
>     @@ -356,7 +356,7 @@ Adding support for a new syntax check tool
>      @code{init-function} is required to initialize the syntax check,
>      usually by creating a temporary copy of the buffer contents.  The
>      function must return @code{(list cmd-name arg-list)}.  If
>     -@code{init-function} returns null, syntax check is aborted, by
>     +@code{init-function} returns null, syntax check is aborted, but
>      @code{flymake-mode} is not switched off.
>
>      @item cleanup-function
>     diff --git a/lisp/progmodes/flymake.el b/lisp/progmodes/flymake.el
>     index 0b28dc31cf..1e959d884c 100644
>     --- a/lisp/progmodes/flymake.el
>     +++ b/lisp/progmodes/flymake.el
>     @@ -335,7 +335,7 @@ flymake-diagnostic-functions
>      * the remaining arguments are keyword-value pairs in the
>        form (:KEY VALUE :KEY2 VALUE2...).  Currently, Flymake provides
>        no such arguments, but backend functions must be prepared to
>     -  accept to accept and possibly ignore any number of them.
>     +  accept and possibly ignore any number of them.
>
>      Backend functions are expected to initiate the buffer check, but
>      aren't required to complete it check before exiting: if the
>
> ciao, lele.
> --
> nickname: Lele Gaifax | Quando vivrò di quello che ho pensato ieri
> real: Emanuele Gaifas | comincerò ad aver paura di chi mi copia.
> lele@metapensiero.it  |                 -- Fortunato Depero, 1929.
>
>
>


-- 
João Távora

[-- Attachment #2: Type: text/html, Size: 4137 bytes --]

Re: New Flymake rewrite in emacs-26

[Lele Gaifax]

João Távora <joaotavora@gmail.com> writes:

> I wonder if you've read the docstrings in the .el files, too, since that's
> where most of the new documentation is right now.

Yes, and I will be doing that until I figure out the details about replacing
my current flymake-python-pyflakes based configuration with a new "native"
backend.

>
>> Will make a proper bug report with a patch, if you prefer:
>
> Neither of those is essential I think. Unless someone disagrees, keep
> posting to this thread and I'll make a commit crediting you as the author.

Great, will do.

Thanks a lot,
ciao, lele.
-- 
nickname: Lele Gaifax | Quando vivrò di quello che ho pensato ieri
real: Emanuele Gaifas | comincerò ad aver paura di chi mi copia.
lele@metapensiero.it  |                 -- Fortunato Depero, 1929.

Re: New Flymake rewrite in emacs-26

[Lele Gaifax]

Lele Gaifax <lele@metapensiero.it> writes:

> João Távora <joaotavora@gmail.com> writes:
>
>> I wonder if you've read the docstrings in the .el files, too, since that's
>> where most of the new documentation is right now.
>
> Yes, and I will be doing that until I figure out the details about replacing
> my current flymake-python-pyflakes based configuration with a new "native"
> backend.

Speaking of which, while reading the -proc source I found recipes for Perl and
PHP, is there any interest in out-of-the-box Python equivalents?

Here is an updated diff: there is a "[???]" marker for a sentence that seems
wrong but I could not understand exactly its meaning.

ciao, lele.

diff --git a/doc/misc/flymake.texi b/doc/misc/flymake.texi
index 5dd72f81e2..5ff5537d04 100644
--- a/doc/misc/flymake.texi
+++ b/doc/misc/flymake.texi
@@ -182,7 +182,7 @@ Syntax check statuses
 @cindex Syntax check statuses

 After syntax check is finished, its status is displayed in the mode line.
-The following statuses are defined.
+The following statuses are defined:

 @multitable @columnfractions 0.25 0.75
 @item @code{Wait}
@@ -356,7 +356,7 @@ Adding support for a new syntax check tool
 @code{init-function} is required to initialize the syntax check,
 usually by creating a temporary copy of the buffer contents.  The
 function must return @code{(list cmd-name arg-list)}.  If
-@code{init-function} returns null, syntax check is aborted, by
+@code{init-function} returns null, syntax check is aborted, but
 @code{flymake-mode} is not switched off.

 @item cleanup-function
@@ -375,7 +375,7 @@ Adding support for a new syntax check tool
 @end table

 To add support for a new syntax check tool, write corresponding
-@code{init-function}, and, optionally @code{cleanup-function} and
+@code{init-function} and, optionally, @code{cleanup-function} and
 @code{getfname-function}.  If the format of error messages reported by
 the new tool is not yet supported by Flymake, add a new entry to
 the @code{flymake-proc-err-line-patterns} list.
@@ -493,7 +493,7 @@ Implementation overview

 @code{flymake-proc-legacy-backend} saves a copy of the buffer in a
 temporary file in the buffer's directory (or in the system temp
-directory, for java files), creates a syntax check command and
+directory, for Java files), creates a syntax check command and
 launches a process with this command.  The output is parsed using a
 list of error message patterns, and error information (file name, line
 number, type and text) is saved.  After the process has finished,
@@ -631,7 +631,7 @@ Locating the buildfile
 using some build tool, like Make or Ant.

 All Make configuration data is usually stored in a file called
-@code{Makefile}.  To allow for future extensions, flymake uses a notion of
+@code{Makefile}.  To allow for future extensions, Flymake uses a notion of
 buildfile to reference the 'project configuration' file.

 Special function, @code{flymake-proc-find-buildfile} is provided for locating buildfiles.
diff --git a/lisp/progmodes/flymake-proc.el b/lisp/progmodes/flymake-proc.el
index 47ec27f611..766dae312c 100644
--- a/lisp/progmodes/flymake-proc.el
+++ b/lisp/progmodes/flymake-proc.el
@@ -113,7 +113,7 @@ flymake-proc--current-process
   "Currently active Flymake process for a buffer, if any.")

 (defvar flymake-proc--report-fn nil
-  "If bound, function used to report back to flymake's UI.")
+  "If bound, function used to report back to Flymake's UI.")

 (defun flymake-proc-reformat-err-line-patterns-from-compile-el (original-list)
   "Grab error line patterns from ORIGINAL-LIST in compile.el format.
@@ -265,7 +265,6 @@ flymake-proc--included-file-name

 (defun flymake-proc--find-possible-master-files (file-name master-file-dirs masks)
   "Find (by name and location) all possible master files.
-
 Name is specified by FILE-NAME and location is specified by
 MASTER-FILE-DIRS.  Master files include .cpp and .c for .h.
 Files are searched for starting from the .h directory and max
@@ -626,7 +625,7 @@ flymake-proc--process-sentinel
 (defun flymake-proc--panic (problem explanation)
   "Tell Flymake UI about a fatal PROBLEM with this backend.
 May only be called in a dynamic environment where
-`flymake-proc--dynamic-report-fn' is bound"
+`flymake-proc--dynamic-report-fn' is bound."
   (flymake-log 0 "%s: %s" problem explanation)
   (if (and (boundp 'flymake-proc--report-fn)
            flymake-proc--report-fn)
diff --git a/lisp/progmodes/flymake.el b/lisp/progmodes/flymake.el
index 0b28dc31cf..003cf93acf 100644
--- a/lisp/progmodes/flymake.el
+++ b/lisp/progmodes/flymake.el
@@ -198,7 +198,7 @@ flymake-log
     `(flymake--log-1 ,level ',sublog ,msg ,@args)))

 (defun flymake-error (text &rest args)
-  "Format TEXT with ARGS and signal an error for flymake."
+  "Format TEXT with ARGS and signal an error for Flymake."
   (let ((msg (apply #'format-message text args)))
     (flymake-log :error msg)
     (error (concat "[Flymake] " msg))))
@@ -335,7 +335,7 @@ flymake-diagnostic-functions
 * the remaining arguments are keyword-value pairs in the
   form (:KEY VALUE :KEY2 VALUE2...).  Currently, Flymake provides
   no such arguments, but backend functions must be prepared to
-  accept to accept and possibly ignore any number of them.
+  accept and possibly ignore any number of them.

 Backend functions are expected to initiate the buffer check, but
 aren't required to complete it check before exiting: if the
@@ -378,8 +378,8 @@ flymake-diagnostic-functions
 * ‘:explanation’: value should give user-readable details of
   the situation encountered, if any.

-* ‘:force’: value should be a boolean suggesting that the Flymake
-  considers the report even if was somehow unexpected.")
+* ‘:force’: value should be a boolean suggesting that Flymake
+  considers the report even if it was somehow unexpected.")

 (defvar flymake-diagnostic-types-alist
   `((:error
@@ -396,7 +396,7 @@ flymake-diagnostic-types-alist

 * Every property pertaining to overlays, except `category' and
   `evaporate' (see Info Node `(elisp)Overlay Properties'), used
-  affect the appearance of Flymake annotations.
+  to affect the appearance of Flymake annotations.

 * `bitmap', an image displayed in the fringe according to
   `flymake-fringe-indicator-position'.  The value actually
@@ -515,12 +515,12 @@ flymake--backend-state
   "Buffer-local hash table of a Flymake backend's state.
 The keys to this hash table are functions as found in
 `flymake-diagnostic-functions'. The values are structures
-of the type `flymake--backend-state', with these slots
+of the type `flymake--backend-state', with these slots:

 `running', a symbol to keep track of a backend's replies via its
 REPORT-FN argument. A backend is running if this key is
 present. If the key is absent if the backend isn't expecting any
-replies from the backend.
+replies from the backend. [???]

 `diags', a (possibly empty) list of diagnostic objects created
 with `flymake-make-diagnostic'. This key is absent if the
@@ -556,7 +556,6 @@ flymake-is-running
                                           &key explanation force
                                           &allow-other-keys)
   "Handle reports from BACKEND identified by TOKEN.
-
 BACKEND, REPORT-ACTION and EXPLANATION, and FORCE conform to the calling
 convention described in `flymake-diagnostic-functions' (which
 see). Optional FORCE says to handle a report even if TOKEN was
@@ -643,7 +642,7 @@ flymake-reporting-backends

 (defun flymake--disable-backend (backend &optional explanation)
   "Disable BACKEND because EXPLANATION.
-If is is running also stop it."
+If it is running also stop it."
   (flymake-log :warning "Disabling backend %s because %s" backend explanation)
   (flymake--with-backend-state backend state
     (setf (flymake--backend-state-running state) nil
@@ -659,7 +658,7 @@ flymake--run-backend
             (flymake--backend-state-disabled state) nil
             (flymake--backend-state-diags state) nil
             (flymake--backend-state-reported-p state) nil))
-    ;; FIXME: Should use `condition-case-unless-debug' here, for don't
+    ;; FIXME: Should use `condition-case-unless-debug' here, it doesn't
     ;; for two reasons: (1) that won't let me catch errors from inside
     ;; `ert-deftest' where `debug-on-error' appears to be always
     ;; t. (2) In cases where the user is debugging elisp somewhere
@@ -799,7 +798,6 @@ flymake-find-file-hook

 (defun flymake-goto-next-error (&optional n filter interactive)
   "Go to Nth next Flymake error in buffer matching FILTER.
-
 Interactively, always move to the next error.  Interactively, and
 with a prefix arg, skip any diagnostics with a severity less than
 ‘:warning’.
@@ -855,7 +853,6 @@ flymake-goto-next-error

 (defun flymake-goto-prev-error (&optional n filter interactive)
   "Go to Nth previous Flymake error in buffer matching FILTER.
-
 Interactively, always move to the previous error.  Interactively,
 and with a prefix arg, skip any diagnostics with a severity less
 than ‘:warning’.

-- 
nickname: Lele Gaifax | Quando vivrò di quello che ho pensato ieri
real: Emanuele Gaifas | comincerò ad aver paura di chi mi copia.
lele@metapensiero.it  |                 -- Fortunato Depero, 1929.

Re: New Flymake rewrite in emacs-26

[João Távora]

Lele Gaifax <lele@metapensiero.it> writes:

> Speaking of which, while reading the -proc source I found recipes for Perl and
> PHP, is there any interest in out-of-the-box Python equivalents?

Very much so, but please not based on flymake-proc.el, rather the new
API.

If your Python interpreter/compiler provides a syntax check tool that
can read from stdin, such a thing can be done in little code.

For an example, see my reply to Simen Heggestøyl in a parallel thread
where I provide a simple but effective Ruby backend.

> Here is an updated diff: there is a "[???]" marker for a sentence that seems
> wrong but I could not understand exactly its meaning.

Thanks, there are many more errors where you found the "[???]". I fixed
them in this diff so you can add them to your list.

diff --git a/lisp/progmodes/flymake.el b/lisp/progmodes/flymake.el
index acc0637ec3..3fd99f8eb3 100644
--- a/lisp/progmodes/flymake.el
+++ b/lisp/progmodes/flymake.el
@@ -526,19 +526,18 @@ flymake--backend-state
 
 `running', a symbol to keep track of a backend's replies via its
 REPORT-FN argument. A backend is running if this key is
-present. If the key is absent if the backend isn't expecting any
-replies from the backend.
+present. If nil, Flymake isn't expecting any replies from the
+backend.
 
-`diags', a (possibly empty) list of diagnostic objects created
-with `flymake-make-diagnostic'. This key is absent if the
-backend hasn't reported anything yet.
+`diags', a (possibly empty) list recent diagnostic objects created
+by the backend with `flymake-make-diagnostic'.
 
 `reported-p', a boolean indicating if the backend has replied
 since it last was contacted.
 
 `disabled', a string with the explanation for a previous
-exceptional situation reported by the backend. If this key is
-present the backend is disabled.")
+exceptional situation reported by the backend, nil if the
+backend is operating normally.")
 
 (cl-defstruct (flymake--backend-state
                (:constructor flymake--make-backend-state))

Re: New Flymake rewrite in emacs-26

[Lele Gaifax]

[-- Attachment #1: Type: text/plain, Size: 1351 bytes --]

joaotavora@gmail.com (João Távora) writes:

> Lele Gaifax <lele@metapensiero.it> writes:
>
>> Speaking of which, while reading the -proc source I found recipes for Perl and
>> PHP, is there any interest in out-of-the-box Python equivalents?
>
> Very much so, but please not based on flymake-proc.el, rather the new
> API.
>
> If your Python interpreter/compiler provides a syntax check tool that
> can read from stdin, such a thing can be done in little code.
>
> For an example, see my reply to Simen Heggestøyl in a parallel thread
> where I provide a simple but effective Ruby backend.

Thank you, I posted the solution I'm currently testing in that thread: let me
know if/how can I contribute it (removing my "esk/" prefix, of course).

I'm attaching the copy-edits diff: it includes your suggested rephrase.

I found the docstrings for flymake-goto-{next,previous}-error somewhat
confusing:

  (defun flymake-goto-next-error (&optional n filter interactive)
    "Go to Nth next Flymake error in buffer matching FILTER.
  Interactively, always move to the next error.  Interactively, and
  with a prefix arg, skip any diagnostics with a severity less than
  ‘:warning’.
  ...

with that double reference to "interactively", but I didn't touch those, as it
may very well be my inability to get the right sense...

Thanks again,
ciao, lele.


[-- Attachment #2: flymake.diff --]
[-- Type: text/x-diff, Size: 9527 bytes --]

diff --git a/doc/misc/flymake.texi b/doc/misc/flymake.texi
index 5dd72f81e2..5ff5537d04 100644
--- a/doc/misc/flymake.texi
+++ b/doc/misc/flymake.texi
@@ -182,7 +182,7 @@ Syntax check statuses
 @cindex Syntax check statuses
 
 After syntax check is finished, its status is displayed in the mode line.
-The following statuses are defined.
+The following statuses are defined:
 
 @multitable @columnfractions 0.25 0.75
 @item @code{Wait}
@@ -356,7 +356,7 @@ Adding support for a new syntax check tool
 @code{init-function} is required to initialize the syntax check,
 usually by creating a temporary copy of the buffer contents.  The
 function must return @code{(list cmd-name arg-list)}.  If
-@code{init-function} returns null, syntax check is aborted, by
+@code{init-function} returns null, syntax check is aborted, but
 @code{flymake-mode} is not switched off.
 
 @item cleanup-function
@@ -375,7 +375,7 @@ Adding support for a new syntax check tool
 @end table
 
 To add support for a new syntax check tool, write corresponding
-@code{init-function}, and, optionally @code{cleanup-function} and
+@code{init-function} and, optionally, @code{cleanup-function} and
 @code{getfname-function}.  If the format of error messages reported by
 the new tool is not yet supported by Flymake, add a new entry to
 the @code{flymake-proc-err-line-patterns} list.
@@ -493,7 +493,7 @@ Implementation overview
 
 @code{flymake-proc-legacy-backend} saves a copy of the buffer in a
 temporary file in the buffer's directory (or in the system temp
-directory, for java files), creates a syntax check command and
+directory, for Java files), creates a syntax check command and
 launches a process with this command.  The output is parsed using a
 list of error message patterns, and error information (file name, line
 number, type and text) is saved.  After the process has finished,
@@ -631,7 +631,7 @@ Locating the buildfile
 using some build tool, like Make or Ant.
 
 All Make configuration data is usually stored in a file called
-@code{Makefile}.  To allow for future extensions, flymake uses a notion of
+@code{Makefile}.  To allow for future extensions, Flymake uses a notion of
 buildfile to reference the 'project configuration' file.
 
 Special function, @code{flymake-proc-find-buildfile} is provided for locating buildfiles.
diff --git a/lisp/progmodes/flymake-proc.el b/lisp/progmodes/flymake-proc.el
index d08819713a..4d9dac363f 100644
--- a/lisp/progmodes/flymake-proc.el
+++ b/lisp/progmodes/flymake-proc.el
@@ -113,7 +113,7 @@ flymake-proc--current-process
   "Currently active Flymake process for a buffer, if any.")
 
 (defvar flymake-proc--report-fn nil
-  "If bound, function used to report back to flymake's UI.")
+  "If bound, function used to report back to Flymake's UI.")
 
 (defun flymake-proc-reformat-err-line-patterns-from-compile-el (original-list)
   "Grab error line patterns from ORIGINAL-LIST in compile.el format.
@@ -265,7 +265,6 @@ flymake-proc--included-file-name
 
 (defun flymake-proc--find-possible-master-files (file-name master-file-dirs masks)
   "Find (by name and location) all possible master files.
-
 Name is specified by FILE-NAME and location is specified by
 MASTER-FILE-DIRS.  Master files include .cpp and .c for .h.
 Files are searched for starting from the .h directory and max
@@ -626,7 +625,7 @@ flymake-proc--process-sentinel
 (defun flymake-proc--panic (problem explanation)
   "Tell Flymake UI about a fatal PROBLEM with this backend.
 May only be called in a dynamic environment where
-`flymake-proc--dynamic-report-fn' is bound"
+`flymake-proc--dynamic-report-fn' is bound."
   (flymake-log 0 "%s: %s" problem explanation)
   (if (and (boundp 'flymake-proc--report-fn)
            flymake-proc--report-fn)
@@ -718,7 +717,7 @@ flymake-proc--safe-delete-directory
 (defun flymake-proc-legacy-flymake (report-fn &rest args)
   "Flymake backend based on the original Flymake implementation.
 This function is suitable for inclusion in
-`flymake-diagnostic-types-alist'. For backward compatibility, it
+`flymake-diagnostic-functions'. For backward compatibility, it
 can also be executed interactively independently of
 `flymake-mode'."
   ;; Interactively, behave as if flymake had invoked us through its
diff --git a/lisp/progmodes/flymake.el b/lisp/progmodes/flymake.el
index acc0637ec3..82877eea57 100644
--- a/lisp/progmodes/flymake.el
+++ b/lisp/progmodes/flymake.el
@@ -203,7 +203,7 @@ flymake-log
     `(flymake--log-1 ,level ',sublog ,msg ,@args)))
 
 (defun flymake-error (text &rest args)
-  "Format TEXT with ARGS and signal an error for flymake."
+  "Format TEXT with ARGS and signal an error for Flymake."
   (let ((msg (apply #'format-message text args)))
     (flymake-log :error msg)
     (error (concat "[Flymake] " msg))))
@@ -342,7 +342,7 @@ flymake-diagnostic-functions
 * the remaining arguments are keyword-value pairs in the
   form (:KEY VALUE :KEY2 VALUE2...).  Currently, Flymake provides
   no such arguments, but backend functions must be prepared to
-  accept to accept and possibly ignore any number of them.
+  accept and possibly ignore any number of them.
 
 Backend functions are expected to initiate the buffer check, but
 aren't required to complete it check before exiting: if the
@@ -385,8 +385,8 @@ flymake-diagnostic-functions
 * ‘:explanation’: value should give user-readable details of
   the situation encountered, if any.
 
-* ‘:force’: value should be a boolean suggesting that the Flymake
-  considers the report even if was somehow unexpected.")
+* ‘:force’: value should be a boolean suggesting that Flymake
+  considers the report even if it was somehow unexpected.")
 
 (defvar flymake-diagnostic-types-alist
   `((:error
@@ -403,7 +403,7 @@ flymake-diagnostic-types-alist
 
 * Every property pertaining to overlays, except `category' and
   `evaporate' (see Info Node `(elisp)Overlay Properties'), used
-  affect the appearance of Flymake annotations.
+  to affect the appearance of Flymake annotations.
 
 * `bitmap', an image displayed in the fringe according to
   `flymake-fringe-indicator-position'.  The value actually
@@ -522,23 +522,22 @@ flymake--backend-state
   "Buffer-local hash table of a Flymake backend's state.
 The keys to this hash table are functions as found in
 `flymake-diagnostic-functions'. The values are structures
-of the type `flymake--backend-state', with these slots
+of the type `flymake--backend-state', with these slots:
 
 `running', a symbol to keep track of a backend's replies via its
 REPORT-FN argument. A backend is running if this key is
-present. If the key is absent if the backend isn't expecting any
-replies from the backend.
+present. If nil, Flymake isn't expecting any replies from the
+backend.
 
-`diags', a (possibly empty) list of diagnostic objects created
-with `flymake-make-diagnostic'. This key is absent if the
-backend hasn't reported anything yet.
+`diags', a (possibly empty) list of recent diagnostic objects
+created by the backend with `flymake-make-diagnostic'.
 
 `reported-p', a boolean indicating if the backend has replied
 since it last was contacted.
 
 `disabled', a string with the explanation for a previous
-exceptional situation reported by the backend. If this key is
-present the backend is disabled.")
+exceptional situation reported by the backend, nil if the
+backend is operating normally.")
 
 (cl-defstruct (flymake--backend-state
                (:constructor flymake--make-backend-state))
@@ -563,7 +562,6 @@ flymake-is-running
                                           &key explanation force
                                           &allow-other-keys)
   "Handle reports from BACKEND identified by TOKEN.
-
 BACKEND, REPORT-ACTION and EXPLANATION, and FORCE conform to the calling
 convention described in `flymake-diagnostic-functions' (which
 see). Optional FORCE says to handle a report even if TOKEN was
@@ -650,7 +648,7 @@ flymake-reporting-backends
 
 (defun flymake--disable-backend (backend &optional explanation)
   "Disable BACKEND because EXPLANATION.
-If is is running also stop it."
+If it is running also stop it."
   (flymake-log :warning "Disabling backend %s because %s" backend explanation)
   (flymake--with-backend-state backend state
     (setf (flymake--backend-state-running state) nil
@@ -666,7 +664,7 @@ flymake--run-backend
             (flymake--backend-state-disabled state) nil
             (flymake--backend-state-diags state) nil
             (flymake--backend-state-reported-p state) nil))
-    ;; FIXME: Should use `condition-case-unless-debug' here, for don't
+    ;; FIXME: Should use `condition-case-unless-debug' here, it doesn't
     ;; for two reasons: (1) that won't let me catch errors from inside
     ;; `ert-deftest' where `debug-on-error' appears to be always
     ;; t. (2) In cases where the user is debugging elisp somewhere
@@ -806,7 +804,6 @@ flymake-find-file-hook
 
 (defun flymake-goto-next-error (&optional n filter interactive)
   "Go to Nth next Flymake error in buffer matching FILTER.
-
 Interactively, always move to the next error.  Interactively, and
 with a prefix arg, skip any diagnostics with a severity less than
 ‘:warning’.
@@ -862,7 +859,6 @@ flymake-goto-next-error
 
 (defun flymake-goto-prev-error (&optional n filter interactive)
   "Go to Nth previous Flymake error in buffer matching FILTER.
-
 Interactively, always move to the previous error.  Interactively,
 and with a prefix arg, skip any diagnostics with a severity less
 than ‘:warning’.

[-- Attachment #3: Type: text/plain, Size: 206 bytes --]


-- 
nickname: Lele Gaifax | Quando vivrò di quello che ho pensato ieri
real: Emanuele Gaifas | comincerò ad aver paura di chi mi copia.
lele@metapensiero.it  |                 -- Fortunato Depero, 1929.

Re: New Flymake rewrite in emacs-26

[Noam Postavsky]

On Thu, Oct 5, 2017 at 7:42 AM, Lele Gaifax <lele@metapensiero.it> wrote:
> diff --git a/doc/misc/flymake.texi b/doc/misc/flymake.texi

I found this additional fix necessary to successfully generate the
info files on my out-of-tree build with texinfo 4.13.

diff --git i/doc/misc/flymake.texi w/doc/misc/flymake.texi
index 5dd72f81e2..89a186f2ea 100644
--- i/doc/misc/flymake.texi
+++ w/doc/misc/flymake.texi
@@ -4,7 +4,7 @@
 @set VERSION 0.3
 @set UPDATED April 2004
 @settitle GNU Flymake @value{VERSION}
-@include ../emacs/docstyle.texi
+@include docstyle.texi
 @syncodeindex pg cp
 @comment %**end of header

Re: New Flymake rewrite in emacs-26

[João Távora]

Noam Postavsky <npostavs@users.sourceforge.net> writes:

> On Thu, Oct 5, 2017 at 7:42 AM, Lele Gaifax <lele@metapensiero.it> wrote:
>> diff --git a/doc/misc/flymake.texi b/doc/misc/flymake.texi
>
> I found this additional fix necessary to successfully generate the
> info files on my out-of-tree build with texinfo 4.13.

Hi Noam,

I'm to blame for changing doctyle.texi to ../emacs/docstyle.texi. I
needed it to fix M-x makeinfo-buffer in the flymake.texi, since there is
no doc/misc/docstyle.texi.

It sounded a good idea to me at the time, but I forgot to ask here. What
exactly is an "out-of-tree" build?

João


> diff --git i/doc/misc/flymake.texi w/doc/misc/flymake.texi
> index 5dd72f81e2..89a186f2ea 100644
> --- i/doc/misc/flymake.texi
> +++ w/doc/misc/flymake.texi
> @@ -4,7 +4,7 @@
>  @set VERSION 0.3
>  @set UPDATED April 2004
>  @settitle GNU Flymake @value{VERSION}
> -@include ../emacs/docstyle.texi
> +@include docstyle.texi
>  @syncodeindex pg cp
>  @comment %**end of header

Re: New Flymake rewrite in emacs-26

[Noam Postavsky]

On Fri, Oct 6, 2017 at 9:16 AM, João Távora <joaotavora@gmail.com> wrote:

> I'm to blame for changing doctyle.texi to ../emacs/docstyle.texi. I
> needed it to fix M-x makeinfo-buffer in the flymake.texi, since there is
> no doc/misc/docstyle.texi.
>
> It sounded a good idea to me at the time, but I forgot to ask here. What
> exactly is an "out-of-tree" build?

In short, when you call ./configure and make from a different
directory than the source's toplevel.
Actually, the official term seems to be "Parallel Build Tree"
https://www.gnu.org/software/automake/manual/html_node/VPATH-Builds.html

Anyway, I don't really understand how the @include texi path works, I
guess the makefile sets it up so that just the basename is enough.
Perhaps there is something similar that could be done via
.dir-locals.el for M-x makeinfo-buffer?

Re: New Flymake rewrite in emacs-26

[João Távora]

Noam Postavsky <npostavs@users.sourceforge.net> writes:

> In short, when you call ./configure and make from a different
> directory than the source's toplevel.

Makes sense. Fixed it in 0d0265bf50e190c77e6a06fd677c0114cb8356a6

> Perhaps there is something similar that could be done via
> .dir-locals.el for M-x makeinfo-buffer?

Perhaps, but the useful variable, makeinfo-options isn't safe for that
(which probably a good idea). I don't mind changing it for each session.

[ As an aside, the way it looks right now, with plans for Flymake
integration in major modes, is that its doc shouldn't be in doc/misc/*
anyway, but in the main manuals ]

João

Re: New Flymake rewrite in emacs-26

[Lele Gaifax]

Thanks a lot for the commit, João!

Just a tiny diff this time:

diff --git a/lisp/progmodes/flymake.el b/lisp/progmodes/flymake.el
index 45f0adfeba..38fc501369 100644
--- a/lisp/progmodes/flymake.el
+++ b/lisp/progmodes/flymake.el
@@ -795,7 +795,7 @@ flymake-goto-next-error
 skip any diagnostics with a severity less than ‘:warning’.
 
 If ‘flymake-wrap-around’ is non-nil and no more next errors,
-resumes search from top
+resumes search from top.
 
 FILTER is a list of diagnostic types found in
 `flymake-diagnostic-types-alist', or nil, if no filter is to be


-- 
nickname: Lele Gaifax | Quando vivrò di quello che ho pensato ieri
real: Emanuele Gaifas | comincerò ad aver paura di chi mi copia.
lele@metapensiero.it  |                 -- Fortunato Depero, 1929.

Re: New Flymake rewrite in emacs-26

[João Távora]

Lele Gaifax <lele@metapensiero.it> writes:

> Thanks a lot for the commit, João!
>
> Just a tiny diff this time:
>
> diff --git a/lisp/progmodes/flymake.el b/lisp/progmodes/flymake.el
> index 45f0adfeba..38fc501369 100644
> --- a/lisp/progmodes/flymake.el
> +++ b/lisp/progmodes/flymake.el
> @@ -795,7 +795,7 @@ flymake-goto-next-error
>  skip any diagnostics with a severity less than ‘:warning’.
>  
>  If ‘flymake-wrap-around’ is non-nil and no more next errors,
> -resumes search from top
> +resumes search from top.
>  
>  FILTER is a list of diagnostic types found in
>  `flymake-diagnostic-types-alist', or nil, if no filter is to be

OK, I fixed that when fixing a bug in that function.

Re: New Flymake rewrite in emacs-26

[João Távora]

Eli Zaretskii <eliz@gnu.org> writes:

> We should add some minimal documentation about the new features.

I added this to doc/misc/flymake.texi. It needs proofreading, but
otherwise I consider it a fairly complete description of the new API.

Importantly, a complete annotated example of a working Flymake backend
is presented. This can be a model for future backends.

> This should be in NEWS, perhaps after adjusting the style a bit.

I'm not great at summarizing changes in a NEWS-snazzy manner, but a
patch is attached.  Apart from that, I see these loose ends:

* Are we keeping track of the Flymake backends developed by people?
  Thus far, I think I counted backends for python, ruby, clang
  (minimal), gcc (minimal) and tex.  Am I missing any?

* Should Flymake do something with next-error-function?

* There is a "Flymake diagnostics buffer" sub-feature in
  scratch/flymake-diagnostics-buffer.  It is reasonably stable.  Is it
  OK to merge into emacs-26?

* Shouldn't flymake.texi be split into two parts, one for the Emacs
  manual and another for Emacs Lisp manual?

João

--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1146,7 +1146,19 @@ languages.  (Version 2.1.0 or later of Enchant is required.)
 ** Flymake
 
 +++
-*** Emacs no longer prompts the user before killing Flymake processes on exit.
+*** Flymake has been completely redesigned
+
+Flymake now annotates arbitrary buffer regions, not just lines. It
+supports arbitrary diagnostic types, not not just errors and warnings
+(see variable 'flymake-diagnostic-types-alist').
+
+It also supports multiple simultaneous backends, meaning that you can
+check your buffer from different perspectives (see variable
+'flymake-diagnostic-functions'). Backends for Emacs-lisp mode are
+provided.
+
+The old Flymake behaviour is preserved in the so-called "legacy
+backend", which has been updated to benefit from new UI features.
 
 \f
 * New Modes and Packages in Emacs 26.1

Re: New Flymake rewrite in emacs-26

[Eli Zaretskii]

> From: joaotavora@gmail.com (João Távora)
> Cc: emacs-devel@gnu.org,  sdl.web@gmail.com,  monnier@iro.umontreal.ca
> Date: Tue, 10 Oct 2017 13:25:30 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > We should add some minimal documentation about the new features.
> 
> I added this to doc/misc/flymake.texi. It needs proofreading, but
> otherwise I consider it a fairly complete description of the new API.

Thanks, I've proofread it and made several fixes, please take a look.
(I can explain each change I made, if it is not immediately apparent
from the change itself or the log message.)

> > This should be in NEWS, perhaps after adjusting the style a bit.
> 
> I'm not great at summarizing changes in a NEWS-snazzy manner, but a
> patch is attached.

Looks good, see a couple of minor comments below.

> Apart from that, I see these loose ends:
> 
> * Are we keeping track of the Flymake backends developed by people?

You mean, in the manual?  I'd prefer not to, since such information
quickly becomes obsolete, and keeping it up to date imposes a
maintenance burden we'd better avoided.

If you mean track them somewhere else, please tell.

> * Should Flymake do something with next-error-function?

I thought it already did?

> * There is a "Flymake diagnostics buffer" sub-feature in
>   scratch/flymake-diagnostics-buffer.  It is reasonably stable.  Is it
>   OK to merge into emacs-26?

If it's easy to show a diff for such a merge, please do.

> * Shouldn't flymake.texi be split into two parts, one for the Emacs
>   manual and another for Emacs Lisp manual?

It doesn't seem worth the hassle.  Most users will be programmers
anyway.

> +*** Flymake has been completely redesigned
> +
> +Flymake now annotates arbitrary buffer regions, not just lines. It
> +supports arbitrary diagnostic types, not not just errors and warnings
                                        ^^^
Extra "not".

> +                                 Backends for Emacs-lisp mode are
> +provided.

I think this should be in single.  And the correct spelling is "Emacs
Lisp mode".

Also, please use our convention of keeping 2 spaces between sentences.

Otherwise, please push these changes, and thanks.

Re: New Flymake rewrite in emacs-26

[João Távora]

Eli Zaretskii <eliz@gnu.org> writes:

> Thanks, I've proofread it and made several fixes, please take a look.
> (I can explain each change I made, if it is not immediately apparent
> from the change itself or the log message.)

No need, looks very good, thanks.

>> > This should be in NEWS, perhaps after adjusting the style a bit.
>> 
>> I'm not great at summarizing changes in a NEWS-snazzy manner, but a
>> patch is attached.
>
> Looks good, see a couple of minor comments below.
>
>> Apart from that, I see these loose ends:
>> 
>> * Are we keeping track of the Flymake backends developed by people?
>
> You mean, in the manual?  I'd prefer not to, since such information
> quickly becomes obsolete, and keeping it up to date imposes a
> maintenance burden we'd better avoided.
>
> If you mean track them somewhere else, please tell.

No, I meant generally keep track of the backends that surfaced in this
list and merge them into the relevant major-mode in emacs-26 (in keeping
with your decision that they should be merged if they work reasonably
well).

>> * Should Flymake do something with next-error-function?
>
> I thought it already did?

It doesn't. And I should have said 'next-error' more generally. IIUC the
place for next-error-function is for major modes, which flymake-mode
isn't (but its proposed diagnostics buffer is).

Anyway I think the problem is that next-error will have a hard time (if
it doesn't already) choosing between its "next-error" source: the
compilation, and grep occur buffers, and now the constantly updated list
of Flymake annotations.

>> * There is a "Flymake diagnostics buffer" sub-feature in
>>   scratch/flymake-diagnostics-buffer.  It is reasonably stable.  Is it
>>   OK to merge into emacs-26?
>
> If it's easy to show a diff for such a merge, please do.

Patch is attached (though I don't really understand if you want to see
the diff or rather ensure that a diff is possible and easy to revert if
problems arise)

> It doesn't seem worth the hassle.  Most users will be programmers
> anyway.

Possibly not elisp programmers, but OK.

> Otherwise, please push these [NEWS] changes, and thanks.

Done.

João

diff --git a/lisp/progmodes/flymake.el b/lisp/progmodes/flymake.el
index 4c4d6aef32..6796fc2b76 100644
--- a/lisp/progmodes/flymake.el
+++ b/lisp/progmodes/flymake.el
@@ -605,7 +605,12 @@ flymake-is-running
             (flymake-log :debug "backend %s reported %d diagnostics in %.2f second(s)"
                          backend
                          (length new-diags)
-                         (- (float-time) flymake-check-start-time)))))))))
+                         (- (float-time) flymake-check-start-time)))
+          (when (and (get-buffer (flymake--diagnostics-buffer-name))
+                     (get-buffer-window (flymake--diagnostics-buffer-name))
+                     (null (cl-set-difference (flymake-running-backends)
+                                              (flymake-reporting-backends))))
+            (flymake-show-diagnostics-buffer))))))))
 
 (defun flymake-make-report-fn (backend &optional token)
   "Make a suitable anonymous report function for BACKEND.
@@ -869,6 +874,7 @@ flymake-goto-prev-error
     [ "Go to previous error"  flymake-goto-prev-error t ]
     [ "Check now"             flymake-start t ]
     [ "Go to log buffer"      flymake-switch-to-log-buffer t ]
+    [ "Show error buffer"     flymake-show-diagnostics-buffer t ]
     "--"
     [ "Turn off Flymake"      flymake-mode t ]))
 
@@ -977,6 +983,102 @@ flymake--mode-line-format
                         '(:propertize " "))
              (:propertize "]")))))))
 
+
+;;; Diagnostics buffer
+
+(defvar-local flymake--diagnostics-buffer-source nil)
+
+(defvar flymake-diagnostics-buffer-mode-map
+  (let ((map (make-sparse-keymap)))
+    (define-key map (kbd "RET") 'flymake-goto-diagnostic)
+    (define-key map (kbd "SPC") 'flymake-show-diagnostic)
+    map))
+
+(defun flymake-show-diagnostic (pos &optional other-window)
+  "Show location of diagnostic at POS."
+  (interactive (list (point) t))
+  (let* ((id (or (tabulated-list-get-id pos)
+                 (user-error "Nothing at point")))
+         (overlay (plist-get id :overlay)))
+    (with-current-buffer (overlay-buffer overlay)
+      (with-selected-window
+          (display-buffer (current-buffer) other-window)
+        (goto-char (overlay-start overlay))
+        (pulse-momentary-highlight-region (overlay-start overlay)
+                                          (overlay-end overlay)
+                                          'highlight))
+      (current-buffer))))
+
+(defun flymake-goto-diagnostic (pos)
+  "Show location of diagnostic at POS.
+POS can be a buffer position or a button"
+  (interactive "d")
+  (pop-to-buffer
+   (flymake-show-diagnostic (if (button-type pos) (button-start pos) pos))))
+
+(defun flymake--diagnostics-buffer-entries ()
+  (with-current-buffer flymake--diagnostics-buffer-source
+    (cl-loop for ov in (flymake--overlays)
+             for diag = (overlay-get ov
+                                     'flymake--diagnostic)
+             for (line . col) =
+             (save-excursion
+               (goto-char (overlay-start ov))
+               (cons (line-number-at-pos)
+                     (- (point)
+                        (line-beginning-position))))
+             for type = (flymake--diag-type diag)
+             collect
+             (list (list :overlay ov
+                         :line line
+                         :severity (flymake--lookup-type-property
+                                    type
+                                    'severity (warning-numeric-level :error)))
+                   `[,(format "%s" line)
+                     ,(format "%s" col)
+                     ,(propertize (format "%s" type)
+                                  'face (flymake--lookup-type-property
+                                         type 'mode-line-face 'flymake-error))
+                     (,(format "%s" (flymake--diag-text diag))
+                      mouse-face highlight
+                      help-echo "mouse-2: visit this diagnostic"
+                      face nil
+                      mouse-action flymake-goto-diagnostic)]))))
+
+(define-derived-mode flymake-diagnostics-buffer-mode tabulated-list-mode
+  "Flymake diagnostics"
+  "A mode for listing Flymake diagnostics."
+  (setq tabulated-list-format
+        `[("Line" 5 (lambda (l1 l2)
+                      (< (plist-get (car l1) :line)
+                         (plist-get (car l2) :line)))
+           :right-align t)
+          ("Col" 3 nil :right-align t)
+          ("Type" 8 (lambda (l1 l2)
+                      (< (plist-get (car l1) :severity)
+                         (plist-get (car l2) :severity))))
+          ("Message" 0 t)])
+  (setq tabulated-list-entries
+        'flymake--diagnostics-buffer-entries)
+  (tabulated-list-init-header))
+
+(defun flymake--diagnostics-buffer-name ()
+  (format "*Flymake diagnostics for %s*" (current-buffer)))
+
+(defun flymake-show-diagnostics-buffer ()
+  "Show a list of Flymake diagnostics for current buffer."
+  (interactive)
+  (let* ((name (flymake--diagnostics-buffer-name))
+         (source (current-buffer))
+         (target (or (get-buffer name)
+                     (with-current-buffer (get-buffer-create name)
+                       (flymake-diagnostics-buffer-mode)
+                       (setq flymake--diagnostics-buffer-source source)
+                       (current-buffer)))))
+    (with-current-buffer target
+      (revert-buffer)
+      (display-buffer (current-buffer)))))
+
 (provide 'flymake)
 
 (require 'flymake-proc)

Re: New Flymake rewrite in emacs-26

[Eli Zaretskii]

> From: joaotavora@gmail.com (João Távora)
> Cc: emacs-devel@gnu.org,  sdl.web@gmail.com,  monnier@iro.umontreal.ca
> Date: Tue, 10 Oct 2017 16:09:07 +0100
> 
> No, I meant generally keep track of the backends that surfaced in this
> list and merge them into the relevant major-mode in emacs-26 (in keeping
> with your decision that they should be merged if they work reasonably
> well).

I think yes.

> >> * Should Flymake do something with next-error-function?
> >
> > I thought it already did?
> 
> It doesn't. And I should have said 'next-error' more generally. IIUC the
> place for next-error-function is for major modes, which flymake-mode
> isn't (but its proposed diagnostics buffer is).

I have no problems with Flymake keeping its hands off next-error.  But
since you've asked the question, it sounds like you are unsure whether
it's TRT?

> Anyway I think the problem is that next-error will have a hard time (if
> it doesn't already) choosing between its "next-error" source: the
> compilation, and grep occur buffers, and now the constantly updated list
> of Flymake annotations.

Yes, we had similar problems elsewhere.

> >> * There is a "Flymake diagnostics buffer" sub-feature in
> >>   scratch/flymake-diagnostics-buffer.  It is reasonably stable.  Is it
> >>   OK to merge into emacs-26?
> >
> > If it's easy to show a diff for such a merge, please do.
> 
> Patch is attached (though I don't really understand if you want to see
> the diff or rather ensure that a diff is possible and easy to revert if
> problems arise)

(I wanted to see the diffs themselves.)

Thanks, I think this can be merged to emacs-26.

> > It doesn't seem worth the hassle.  Most users will be programmers
> > anyway.
> 
> Possibly not elisp programmers, but OK.

Let's withhold the argument until we have some real problems in this
area.  IMO, the manual is small enough to host both parts of Flymake
documentation without any problem.

Re: New Flymake rewrite in emacs-26

[João Távora]

Eli Zaretskii <eliz@gnu.org> writes:

> I think yes.

OK, IIUC we have copyright assignments for every author besides me
(that’s Lele and Mark). So I invite these people to cross-check their
work against new API documentation, and merge it to emacs-26.

It would be nice to have a backend capable of checking Emacs C sources
with GCC and without any extra configurations. To solve the problem with
setting proper GCC flags, perhaps the .dir-locals file can be used. Or
perhaps some other method can be used to infer flags from a Makefile or
some other source.

Likewise for a perl backend, which should be even simpler.

>> >> * Should Flymake do something with next-error-function?
>> >
>> > I thought it already did?
>> 
>> It doesn't. And I should have said 'next-error' more generally. IIUC the
>> place for next-error-function is for major modes, which flymake-mode
>> isn't (but its proposed diagnostics buffer is).
>
> I have no problems with Flymake keeping its hands off next-error.  But
> since you've asked the question, it sounds like you are unsure whether
> it's TRT?

Yes. I don’t use next-error at all, so I don’t think I’m a good person
to implement this kind of UI integration (rather, I’ll implement it 
iff someone provides precise requires).

> Thanks, I think this can be merged to emacs-26.

Done

>> Possibly not elisp programmers, but OK.
>
> Let's withhold the argument until we have some real problems in this
> area.  IMO, the manual is small enough to host both parts of Flymake
> documentation without any problem.

Mainly I thought it would increase awareness of the Flymake API to
major-mode-writers, but I agree with your argument.

Re: New Flymake rewrite in emacs-26

[Eli Zaretskii]

> From: joaotavora@gmail.com (João Távora)
> Cc: emacs-devel@gnu.org,  sdl.web@gmail.com,  monnier@iro.umontreal.ca,
> 	Mark Oteiza <mvoteiza@udel.edu>, Lele Gaifax <lele@metapensiero.it>
> Date: Tue, 10 Oct 2017 17:25:44 +0100
> 
> > I think yes.
> 
> OK, IIUC we have copyright assignments for every author besides me

If you are unsure, I can check.

> It would be nice to have a backend capable of checking Emacs C sources
> with GCC and without any extra configurations. To solve the problem with
> setting proper GCC flags, perhaps the .dir-locals file can be used. Or
> perhaps some other method can be used to infer flags from a Makefile or
> some other source.

Most of the flags we use are for flagging dubious or erroneous code,
they should not matter, I think.  What compiler flags must be set for
Flycheck to work reasonably well with a program's sources?  I guess
the -I flags, but what else?  (I have never used Flymake, so my
questions are ones of a layman in this matter.)

Re: New Flymake rewrite in emacs-26

[João Távora]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: joaotavora@gmail.com (João Távora)
>> Cc: emacs-devel@gnu.org,  sdl.web@gmail.com,  monnier@iro.umontreal.ca,
>> 	Mark Oteiza <mvoteiza@udel.edu>, Lele Gaifax <lele@metapensiero.it>
>> Date: Tue, 10 Oct 2017 17:25:44 +0100
>> 
>> > I think yes.
>> 
>> OK, IIUC we have copyright assignments for every author besides me
>
> If you are unsure, I can check.

No need I think (Lele Gaifax and Marl Oteiza are two contributors
involved)

> Most of the flags we use are for flagging dubious or erroneous code,
> they should not matter, I think.

I think it’s useful to have Flymake synchronized with these flags, so
that you are notified visually of these problems ahead of a "real"
compilation. FWIW this is what the Emacs Lisp backend already does for
Emacs Lisp sources.

> What compiler flags must be set for Flycheck to work reasonably well
> with a program's sources?  I guess the -I flags, but what else?

For "reasonable" operation, it’s mostly the -I flags, and maybe some
essential -D flags (In Emacs’s case -Demacs, I think).

> (I have never used Flymake, so my questions are ones of a layman in

Fittingly, this is what a sufficiently smart GCC backend could help
change.

Re: New Flymake rewrite in emacs-26

[Noam Postavsky]

On Tue, Oct 10, 2017 at 1:03 PM, João Távora <joaotavora@gmail.com> wrote:
> Eli Zaretskii <eliz@gnu.org> writes:
>
>> Most of the flags we use are for flagging dubious or erroneous code,
>> they should not matter, I think.
>
> I think it’s useful to have Flymake synchronized with these flags, so
> that you are notified visually of these problems ahead of a "real"
> compilation. FWIW this is what the Emacs Lisp backend already does for
> Emacs Lisp sources.
>
>> What compiler flags must be set for Flycheck to work reasonably well
>> with a program's sources?  I guess the -I flags, but what else?
>
> For "reasonable" operation, it’s mostly the -I flags, and maybe some
> essential -D flags (In Emacs’s case -Demacs, I think).
>
>> (I have never used Flymake, so my questions are ones of a layman in

I've also hardly ever used flymake, so this may be nonsense, but maybe
you could run 'make --just-print foo.o' to get the exact shell
commands for compiling foo.c. This would bypass any need to know
details of compilation beyond figuring out the target name from the
source.

Re: New Flymake rewrite in emacs-26

[Eli Zaretskii]

> From: joaotavora@gmail.com (João Távora)
> Date: Tue, 10 Oct 2017 18:03:19 +0100
> Cc: mvoteiza@udel.edu, lele@metapensiero.it, sdl.web@gmail.com,
> 	monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> > Most of the flags we use are for flagging dubious or erroneous code,
> > they should not matter, I think.
> 
> I think it’s useful to have Flymake synchronized with these flags, so
> that you are notified visually of these problems ahead of a "real"
> compilation.

Then just pick the flags up from src/Makefile.

Re: New Flymake rewrite in emacs-26

[João Távora]

Noam Postavsky <npostavs@users.sourceforge.net> writes:

> I've also hardly ever used flymake, so this may be nonsense, but maybe
> you could run 'make --just-print foo.o' to get the exact shell
> commands for compiling foo.c. This would bypass any need to know
> details of compilation beyond figuring out the target name from the
> source.

Thanks for the idea. Certainly not nonsense, but also not a silver
bullet, since Makefiles can invoke gcc in arbitrary ways that fool a
guesser. Still, it’s probably decent in a fair amount of cases, and I’m
giving it a go to see if it works with Emacs sources and perhaps some
other GNU projects. I attach my flag-guessing function at the end.

First, my idea is to cache the result of these flags contingent on the
Makefile’s location and mtime. This, I think, is doable. Then I use a
regexp to extract the gcc invocation from the output.

The regexp is very poor but does the job. For the src/fringe.c file the
regexp is

   "gcc[[:space:]]+\\(\\(?:-.*\\)*\\)/path/to/fringe.c"

This indeed matches Make’s output and gets me something like this for
the match group 1

   -c -Demacs -I. -I. -I../lib -I../lib -isystem /usr/include/freetype2
    -isystem /usr/include/alsa -pthread -isystem /usr/include/librsvg-2.0
    -isystem /usr/include/gdk-pixbuf-2.0 -isystem /usr/include/libpng16
    -isystem /usr/include/cairo -isystem /usr/include/glib-2.0 -isystem
    /usr/lib/x86_64-linux-gnu/glib-2.0/include -isystem
    /usr/include/pixman-1 -isystem /usr/include/freetype2 -isystem
    /usr/include/libpng16 -isystem /usr/include/libpng16 -isystem
    /usr/include/libxml2 -isystem /usr/include/dbus-1.0 -isystem
    /usr/lib/x86_64-linux-gnu/dbus-1.0/include -pthread -isystem
    /usr/include/glib-2.0 -isystem
    /usr/lib/x86_64-linux-gnu/glib-2.0/include -isystem
    /usr/include/glib-2.0 -isystem
    /usr/lib/x86_64-linux-gnu/glib-2.0/include -isystem
    /usr/include/freetype2 -isystem /usr/include/freetype2 -isystem
    /usr/include/freetype2 -MMD -MF
    deps//home/capitaomorte/Source/Emacs/emacs-26/src/fringe.d -MP -isystem
    /usr/include/p11-kit-1 -fno-common -W -Wabi -Waddress
    [... elided many many -W flags]
    -Wno-type-limits -Wno-unused-parameter -Wno-format-nonliteral -g3 -O2

Unfortunately, not all flags make sense for flymake, like the -M family
of flags. Ideally, i’d need a way to parse this big string of flags back
into, say, an alist, and cherry pick the -I, -D, and -W flags from that
set. But I’m afraid split-string will insufficiently deal with escaped
spaces in the output.

Any ideas?

João

PS: My WIP follows

(defvar-local flymake--gcc-cached-flags nil)

(defun flymake-gcc-guess-gcc-flags (&optional trash-cache)
  (interactive "P")
  (unless (executable-find "make")
    (error "Cannot find a suitable make"))
  (when trash-cache
    (setq flymake--gcc-cached-flags nil))
  (catch 'retval
    (when-let* ((makefile-dir
                 (locate-dominating-file default-directory "Makefile"))
                (makefile (expand-file-name "Makefile" makefile-dir))
                (mtime (file-attribute-modification-time
                        (file-attributes makefile))))
      (cond
       ((equal (list makefile mtime)
               (cdr flymake--gcc-cached-flags))
        (when (called-interactively-p 'interactive)
          (message "cached hit for flags for this buffer: %s"
                   (car flymake--gcc-cached-flags)))
        (throw 'retval (car flymake--gcc-cached-flags)))
       (t
        (when-let*
            ((blob (shell-command-to-string
                    (format "make -B -C %s -f %s --just-print %s.o"
                            makefile-dir
                            makefile
                            (file-name-sans-extension (buffer-file-name)))))
             (match (string-match
                     (format "gcc[[:space:]]+\\(\\(?:-.*\\)*\\)%s"
                             (buffer-file-name))
                     blob))
             (flags (split-string (match-string 1 blob) nil t "[[:space:]]")))
          (setq flymake--gcc-cached-flags (list flags makefile mtime))
          (when (called-interactively-p 'interactive)
            (message "cached miss for flags for this buffer: %s" flags))
          (throw 'retval flags))))
      (when (called-interactively-p 'interactive)
        (message "Couldn’t guess flags for this buffer :-(")))))

Re: New Flymake rewrite in emacs-26

[Noam Postavsky]

On Tue, Oct 10, 2017 at 8:07 PM, João Távora <joaotavora@gmail.com> wrote:

> Unfortunately, not all flags make sense for flymake, like the -M family
> of flags.

Oh, huh. I hadn't thought of that. Do the -M flags do any harm though?

> Ideally, i’d need a way to parse this big string of flags back
> into, say, an alist, and cherry pick the -I, -D, and -W flags from that
> set. But I’m afraid split-string will insufficiently deal with escaped
> spaces in the output.

There was some talk of extracting the eshell code into a separate
function that could provide "real" shell parsing. The downside of this
approach is that it would take some work (eshell code is generally
kind of hairy), and it will probably never be 100% since the actual
Makefile isn't using eshell to parse.

Another possibility is to put an echoing command in front and exploit
the shell's parsing directly, e.g.,

  printf '%s\0' gcc -c -Demacs -I. -I. -I../lib -I../ ...

should give you some output that you can split-string on NUL without
worry. Instead of printf, which may not be available, you could use
emacs.

Re: New Flymake rewrite in emacs-26

[Eli Zaretskii]

> From: joaotavora@gmail.com (João Távora)
> Cc: Eli Zaretskii <eliz@gnu.org>,  Mark Oteiza <mvoteiza@udel.edu>,  Lele Gaifax <lele@metapensiero.it>,  Leo Liu <sdl.web@gmail.com>,  Stefan Monnier <monnier@iro.umontreal.ca>,  Emacs developers <emacs-devel@gnu.org>
> Date: Wed, 11 Oct 2017 01:07:19 +0100
> 
> Thanks for the idea. Certainly not nonsense, but also not a silver
> bullet, since Makefiles can invoke gcc in arbitrary ways that fool a
> guesser. Still, it’s probably decent in a fair amount of cases, and I’m
> giving it a go to see if it works with Emacs sources and perhaps some
> other GNU projects. I attach my flag-guessing function at the end.
> 
> First, my idea is to cache the result of these flags contingent on the
> Makefile’s location and mtime. This, I think, is doable. Then I use a
> regexp to extract the gcc invocation from the output.
> 
> The regexp is very poor but does the job. For the src/fringe.c file the
> regexp is
> 
>    "gcc[[:space:]]+\\(\\(?:-.*\\)*\\)/path/to/fringe.c"
> 
> This indeed matches Make’s output and gets me something like this for
> the match group 1
> 
>    -c -Demacs -I. -I. -I../lib -I../lib -isystem /usr/include/freetype2
>     -isystem /usr/include/alsa -pthread -isystem /usr/include/librsvg-2.0
>     -isystem /usr/include/gdk-pixbuf-2.0 -isystem /usr/include/libpng16
>     -isystem /usr/include/cairo -isystem /usr/include/glib-2.0 -isystem
>     /usr/lib/x86_64-linux-gnu/glib-2.0/include -isystem
>     /usr/include/pixman-1 -isystem /usr/include/freetype2 -isystem
>     /usr/include/libpng16 -isystem /usr/include/libpng16 -isystem
>     /usr/include/libxml2 -isystem /usr/include/dbus-1.0 -isystem
>     /usr/lib/x86_64-linux-gnu/dbus-1.0/include -pthread -isystem
>     /usr/include/glib-2.0 -isystem
>     /usr/lib/x86_64-linux-gnu/glib-2.0/include -isystem
>     /usr/include/glib-2.0 -isystem
>     /usr/lib/x86_64-linux-gnu/glib-2.0/include -isystem
>     /usr/include/freetype2 -isystem /usr/include/freetype2 -isystem
>     /usr/include/freetype2 -MMD -MF
>     deps//home/capitaomorte/Source/Emacs/emacs-26/src/fringe.d -MP -isystem
>     /usr/include/p11-kit-1 -fno-common -W -Wabi -Waddress
>     [... elided many many -W flags]
>     -Wno-type-limits -Wno-unused-parameter -Wno-format-nonliteral -g3 -O2
> 
> Unfortunately, not all flags make sense for flymake, like the -M family
> of flags. Ideally, i’d need a way to parse this big string of flags back
> into, say, an alist, and cherry pick the -I, -D, and -W flags from that
> set. But I’m afraid split-string will insufficiently deal with escaped
> spaces in the output.
> 
> Any ideas?

We are talking about a solution specific to Emacs, right?  If so, and
assuming that Flymake is already capable of parsing a Makefile, we
could simply have somewhere the list of *FLAGS variables from the
Emacs Makefile that Flymake needs to use.  That list will not include
DEPFLAGS, for example,, which you say make no sense for Flymake.  Then
Flymake could pick up the expanded value of each of these variables,
and concatenate them.

Re: New Flymake rewrite in emacs-26

[Eli Zaretskii]

> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Date: Tue, 10 Oct 2017 20:59:42 -0400
> Cc: Eli Zaretskii <eliz@gnu.org>, Mark Oteiza <mvoteiza@udel.edu>, Lele Gaifax <lele@metapensiero.it>, 
> 	Leo Liu <sdl.web@gmail.com>, Stefan Monnier <monnier@iro.umontreal.ca>, 
> 	Emacs developers <emacs-devel@gnu.org>
> 
> Another possibility is to put an echoing command in front and exploit
> the shell's parsing directly, e.g.,
> 
>   printf '%s\0' gcc -c -Demacs -I. -I. -I../lib -I../ ...

Which shell, though?  If Eshell, fine; but if you mean the system
shell, that could be unportable.

Re: New Flymake rewrite in emacs-26

[Lele Gaifax]

joaotavora@gmail.com (João Távora) writes:

> OK, IIUC we have copyright assignments for every author besides me
> (that’s Lele and Mark). So I invite these people to cross-check their
> work against new API documentation, and merge it to emacs-26.

I'm currently satisfied by my own python-flymake backend, using current
emacs-26 tip (I keep it up-to-date every day, and update my Emacs installation
twice a week or so), but to be on the safe side I will wait some more days
before reattaching it as a [PATCH] against python.el.

ciao, lele.
-- 
nickname: Lele Gaifax | Quando vivrò di quello che ho pensato ieri
real: Emanuele Gaifas | comincerò ad aver paura di chi mi copia.
lele@metapensiero.it  |                 -- Fortunato Depero, 1929.

Re: New Flymake rewrite in emacs-26

[João Távora]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: joaotavora@gmail.com (João Távora)
>> Cc: Eli Zaretskii <eliz@gnu.org>,  Mark Oteiza <mvoteiza@udel.edu>,  Lele Gaifax <lele@metapensiero.it>,  Leo Liu <sdl.web@gmail.com>,  Stefan Monnier <monnier@iro.umontreal.ca>,  Emacs developers <emacs-devel@gnu.org>
>> Date: Wed, 11 Oct 2017 01:07:19 +0100
>> 
>> Thanks for the idea. Certainly not nonsense, but also not a silver
>> bullet, since Makefiles can invoke gcc in arbitrary ways that fool a
>> guesser. Still, it’s probably decent in a fair amount of cases, and I’m
>> giving it a go to see if it works with Emacs sources and perhaps some
>> other GNU projects. I attach my flag-guessing function at the end.
>> 
>> First, my idea is to cache the result of these flags contingent on the
>> Makefile’s location and mtime. This, I think, is doable. Then I use a
>> regexp to extract the gcc invocation from the output.
>> 
>> The regexp is very poor but does the job. For the src/fringe.c file the
>> regexp is
>> 
>>    "gcc[[:space:]]+\\(\\(?:-.*\\)*\\)/path/to/fringe.c"
>> 
>> This indeed matches Make’s output and gets me something like this for
>> the match group 1
>> 
>>    -c -Demacs -I. -I. -I../lib -I../lib -isystem /usr/include/freetype2
>>     -isystem /usr/include/alsa -pthread -isystem /usr/include/librsvg-2.0
>>     -isystem /usr/include/gdk-pixbuf-2.0 -isystem /usr/include/libpng16
>>     -isystem /usr/include/cairo -isystem /usr/include/glib-2.0 -isystem
>>     /usr/lib/x86_64-linux-gnu/glib-2.0/include -isystem
>>     /usr/include/pixman-1 -isystem /usr/include/freetype2 -isystem
>>     /usr/include/libpng16 -isystem /usr/include/libpng16 -isystem
>>     /usr/include/libxml2 -isystem /usr/include/dbus-1.0 -isystem
>>     /usr/lib/x86_64-linux-gnu/dbus-1.0/include -pthread -isystem
>>     /usr/include/glib-2.0 -isystem
>>     /usr/lib/x86_64-linux-gnu/glib-2.0/include -isystem
>>     /usr/include/glib-2.0 -isystem
>>     /usr/lib/x86_64-linux-gnu/glib-2.0/include -isystem
>>     /usr/include/freetype2 -isystem /usr/include/freetype2 -isystem
>>     /usr/include/freetype2 -MMD -MF
>>     deps//home/capitaomorte/Source/Emacs/emacs-26/src/fringe.d -MP -isystem
>>     /usr/include/p11-kit-1 -fno-common -W -Wabi -Waddress
>>     [... elided many many -W flags]
>>     -Wno-type-limits -Wno-unused-parameter -Wno-format-nonliteral -g3 -O2
>> 
>> Unfortunately, not all flags make sense for flymake, like the -M family
>> of flags. Ideally, i’d need a way to parse this big string of flags back
>> into, say, an alist, and cherry pick the -I, -D, and -W flags from that
>> set. But I’m afraid split-string will insufficiently deal with escaped
>> spaces in the output.
>> 
>> Any ideas?
>
> We are talking about a solution specific to Emacs, right?

No, I was aiming for something more generic that includes at least Emacs
and perhaps otherGNU projects.

But I have no problem in making it work just for Emacs first.

> If so, and assuming that Flymake is already capable of parsing a
> Makefile,

It is not. The only thing it currently knows about Makefiles is what it
has known for many versions, which is to invoke their "check-syntax"
target with a special environment variable set to to a specific
temporary file which contains the buffer's contents. This invocation is
expected to compile files with the correct flags and produce errors and
warnings in the standard output.

> we could simply have somewhere the list of *FLAGS variables from the
> Emacs Makefile that Flymake needs to use.  That list will not include
> DEPFLAGS, for example,, which you say make no sense for Flymake.  Then
> Flymake could pick up the expanded value of each of these variables,
> and concatenate them.

If we are going to make it specific to Emacs, this sounds more
complicated than adding a "check-syntax" target to the Makefile, which
would bring the (minor) benefit that people using emacs < 26.1 could
also use older Flymake to edit Emacs sources.

João

Re: New Flymake rewrite in emacs-26

[Eli Zaretskii]

> From: joaotavora@gmail.com (João Távora)
> Cc: npostavs@users.sourceforge.net,  lele@metapensiero.it,  emacs-devel@gnu.org,  mvoteiza@udel.edu,  monnier@iro.umontreal.ca,  sdl.web@gmail.com
> Date: Wed, 11 Oct 2017 13:01:41 +0100
> 
> If we are going to make it specific to Emacs, this sounds more
> complicated than adding a "check-syntax" target to the Makefile, which
> would bring the (minor) benefit that people using emacs < 26.1 could
> also use older Flymake to edit Emacs sources.

Yes, a special Makefile target is also OK.

Re: New Flymake rewrite in emacs-26

[Noam Postavsky]

On Wed, Oct 11, 2017 at 6:39 AM, Eli Zaretskii <eliz@gnu.org> wrote:

>> Another possibility is to put an echoing command in front and exploit
>> the shell's parsing directly, e.g.,
>>
>>   printf '%s\0' gcc -c -Demacs -I. -I. -I../lib -I../ ...
>
> Which shell, though?  If Eshell, fine; but if you mean the system
> shell, that could be unportable.

Oh right. Maybe we could do 'make -f check-shell.mk get-shell', where
check-shell.mk is:

include Makefile
get-shell: ; echo $(SHELL)

Re: New Flymake rewrite in emacs-26

[João Távora]

Noam Postavsky <npostavs@users.sourceforge.net> writes:

>>> Another possibility is to put an echoing command in front and exploit
>>> the shell's parsing directly, e.g.,
>>>
>>>   printf '%s\0' gcc -c -Demacs -I. -I. -I../lib -I../ ...

I tried with Emacs itself, as you suggested. It works, and may be a
start, but:

* is very slow (only needs to run once though)
* no idea if it works for windows (it should if shell-command DTRT)
* generally looks brittle. Among other things it takes advantage of an
  undocumented "--" command-line arg.

    (defun flymake--gcc-heroic-unescape (string)
      (with-temp-buffer
        (let ((error-buffer (current-buffer)))
          (with-temp-buffer
            (cond
             ((zerop
               (shell-command
                (format
                 "%s -Q --batch --eval \"%s\" -- %s"
                 (expand-file-name invocation-name
                                   invocation-directory)
                 "(mapc 'print (nthcdr 4 command-line-args))"
                 string)
                (current-buffer)
                error-buffer))
              (goto-char (point-min))
              (cl-loop with eof = (make-symbol "eof")
                       for form =
                       (condition-case err
                           (read (current-buffer))
                         (error eof))
                       while (not (eq form eof))
                       collect form))
             (t
              (with-current-buffer error-buffer
                (error (buffer-string)))))))))
     
    (flymake--gcc-heroic-unescape "bla\\ bla") ; ("bla bla")
     
    (flymake-gcc-heroic-unescape "bla bla") ;("bla" "bla")

Re: New Flymake rewrite in emacs-26

[Mark Oteiza]

On 11/10/17 at 01:07am, João Távora wrote:
>    -c -Demacs -I. -I. -I../lib -I../lib  [...]

Do dir-locals do the right thing when you have an entry like

  (c-mode . ((c-flymake-includes . ("." "../lib" ...))))

in Emacs's .dir-locals.el in the project root?

Re: New Flymake rewrite in emacs-26

[João Távora]

joaotavora@gmail.com (João Távora) writes:

> No, I was aiming for something more generic that includes at least Emacs
> and perhaps other GNU projects.

FWIW, here's what I hacked up so far. Seems to work OK in two GNU
projects: Emacs and Hello (after ./configure, of course).

Probably flawed, but it's a start. Feedback welcome. To test, just load
this file and M-x flymake-mode in a C file.

Another option, as suggested previously, is to have a special Makefile
target (that'll need reconfiguring for each project, though).

João

;;; flymake-gcc.el --- naive gcc Flymake backend -*- lexical-binding: t; -*-

(defvar flymake-gcc-program "gcc"
  "GCC program")

(defun flymake--gcc-heroic-unescape (string)
  (with-temp-buffer
    (let ((error-buffer (current-buffer)))
      (with-temp-buffer
        (cond
         ((zerop
           ;; I suspect "shell-command" makes windows even when called
           ;; from lisp.
           (save-window-excursion
             (shell-command
              (format
               "%s -Q --batch --eval \"%s\" -- %s"
               (expand-file-name invocation-name
                                 invocation-directory)
               "(mapc 'print (nthcdr 4 command-line-args))"
               string)
              (current-buffer)
              error-buffer)))
          (goto-char (point-min))
          (cl-loop with eof = (make-symbol "eof")
                   for form =
                   (condition-case _err
                       (read (current-buffer))
                     (error eof))
                   while (not (eq form eof))
                   collect form))
         (t
          (with-current-buffer error-buffer
            (error (buffer-string)))))))))

(defvar flymake-gcc-flags 'flymake-gcc-guess-flags
  "A list of flags passed to GCC.
Alternatively, a symbol naming a function called with no
arguments that should produce this list of flags, or error if it
cannot do so.")

(defvar flymake-gcc-extra-flags '("-Wextra" "-Wall")
  "A list of extra flags passed to GCC.")

(defvar-local flymake--gcc-cached-flags nil
  "Internal variable for `flymake-gcc-guess-flags'")

(defun flymake-gcc-guess-flags (&optional trash-cache)
  "Guess GCC flags for compiling current buffer "
  (interactive "P")
  (unless (executable-find "make") (error "Cannot find a suitable make"))
  (when trash-cache (setq flymake--gcc-cached-flags nil))
  (catch 'retval
    (unless (buffer-file-name)
      ;; don't error and don't cache, so that when the buffer is saved
      ;; we get another chance.
      (throw 'retval nil))
    (when-let* ((makefile-dir
                 (locate-dominating-file default-directory "Makefile"))
                (makefile (expand-file-name "Makefile" makefile-dir))
                (mtime (file-attribute-modification-time
                        (file-attributes makefile))))
      (cond
       ((equal (list makefile mtime)
               (cdr flymake--gcc-cached-flags))
        (when (called-interactively-p 'interactive)
          (message "cached hit for flags for this buffer: %s"
                   (car flymake--gcc-cached-flags)))
        (throw 'retval (car flymake--gcc-cached-flags)))
       (t
        (let*
            ((sans-nothing
              (file-name-nondirectory
               (file-name-sans-extension
                (buffer-file-name))))
             (blob (shell-command-to-string
                    (format "make -C %s -f %s --just-print %s.o"
                            makefile-dir
                            makefile
                            sans-nothing)))
             (match (string-match
                     (format "gcc[[:space:]]+\\(\\(?:-.*\\)*\\)%s"
                             sans-nothing)
                     blob))
             (flag-string (and match
                               (match-string 1 blob)))
             (flags (and flag-string
                         (flymake--gcc-heroic-unescape flag-string))))
          (when (or flags (string= "" flag-string))
            (setq flymake--gcc-cached-flags (list flags makefile mtime))
            (when (called-interactively-p 'interactive)
              (message "cached miss for flags for this buffer: %s" flags))
            (throw 'retval flags))))))
    (error "Could not guess gcc flags")))


(defvar-local flymake--gcc-proc nil
  "Internal variable for `flymake-gcc'")

(defun flymake-gcc (report-fn &rest _args)
  "Flymake backend for GCC"
  (unless (executable-find flymake-gcc-program)
    (error "Cannot find a suitable gcc"))
  (when (process-live-p flymake--gcc-proc)
    (kill-process flymake--gcc-proc))
  (let ((source (current-buffer)))
    (save-restriction
      (widen)
      (setq flymake--gcc-proc
            (make-process
             :name "gcc-flymake"
             :buffer (generate-new-buffer "*gcc-flymake*")
             :command `(,flymake-gcc-program
                        "-fsyntax-only"
                        ,@flymake-gcc-extra-flags
                        ,@(if (symbolp flymake-gcc-flags)
                              (funcall flymake-gcc-flags)
                            flymake-gcc-flags)
                        "-x" "c" "-")
             :noquery t :connection-type 'pipe
             :sentinel
             (lambda (p _ev)
               (when (eq 'exit (process-status p))
                 (unwind-protect
                     (when (eq p flymake--gcc-proc)
                       (with-current-buffer (process-buffer p)
                         (goto-char (point-min))
                         (cl-loop
                          while (search-forward-regexp
                                 "^<stdin>:\\([0-9]+\\):\\([0-9]+\\): \\(.*\\): \\(.*\\)$"
                                 nil t)
                          for msg = (match-string 4)
                          for (beg . end) =
                          (flymake-diag-region
                           source
                           (string-to-number (match-string 1))
                           (string-to-number (match-string 2)))
                          for type = (assoc-default
                                      (match-string 3)
                                      '(("error" . :error)
                                        ("note" . :note)
                                        ("warning" . :warning))
                                      #'string-match)
                          collect (flymake-make-diagnostic source beg end type msg)
                          into diags
                          finally (funcall report-fn diags))))
                   ;; (display-buffer (process-buffer p)) ; use this instead of the next one for debug
                   (kill-buffer (process-buffer p))
                   ))
               )))
      (process-send-region flymake--gcc-proc (point-min) (point-max))
      (process-send-eof flymake--gcc-proc))))

(defun flymake--setup-gcc-flymake ()
  (add-hook 'flymake-diagnostic-functions 'flymake-gcc nil t))

(add-hook 'c-mode-hook 'flymake--setup-gcc-flymake)

;;; flymake-gcc.el ends here

Re: New Flymake rewrite in emacs-26

[Romanos Skiadas]

Hi,

This looks very interesting. I haven't tried it out yet, but the idea 
behind it seems to be very similar to cmake-ide[1]. Would it make sense 
to spin this off in its own library instead of making it flymake 
specific? This way other packages like company/auto-complete/flycheck 
can use it too.

Best,
Romanos

[1][https://github.com/atilaneves/cmake-ide]

On 11/10/17 14:41, João Távora wrote:
> joaotavora@gmail.com (João Távora) writes:
>
>> No, I was aiming for something more generic that includes at least Emacs
>> and perhaps other GNU projects.
> FWIW, here's what I hacked up so far. Seems to work OK in two GNU
> projects: Emacs and Hello (after ./configure, of course).
>
> Probably flawed, but it's a start. Feedback welcome. To test, just load
> this file and M-x flymake-mode in a C file.
>
> Another option, as suggested previously, is to have a special Makefile
> target (that'll need reconfiguring for each project, though).
>
> João
>
> ;;; flymake-gcc.el --- naive gcc Flymake backend -*- lexical-binding: t; -*-
>
> (defvar flymake-gcc-program "gcc"
>    "GCC program")
>
> (defun flymake--gcc-heroic-unescape (string)
>    (with-temp-buffer
>      (let ((error-buffer (current-buffer)))
>        (with-temp-buffer
>          (cond
>           ((zerop
>             ;; I suspect "shell-command" makes windows even when called
>             ;; from lisp.
>             (save-window-excursion
>               (shell-command
>                (format
>                 "%s -Q --batch --eval \"%s\" -- %s"
>                 (expand-file-name invocation-name
>                                   invocation-directory)
>                 "(mapc 'print (nthcdr 4 command-line-args))"
>                 string)
>                (current-buffer)
>                error-buffer)))
>            (goto-char (point-min))
>            (cl-loop with eof = (make-symbol "eof")
>                     for form =
>                     (condition-case _err
>                         (read (current-buffer))
>                       (error eof))
>                     while (not (eq form eof))
>                     collect form))
>           (t
>            (with-current-buffer error-buffer
>              (error (buffer-string)))))))))
>
> (defvar flymake-gcc-flags 'flymake-gcc-guess-flags
>    "A list of flags passed to GCC.
> Alternatively, a symbol naming a function called with no
> arguments that should produce this list of flags, or error if it
> cannot do so.")
>
> (defvar flymake-gcc-extra-flags '("-Wextra" "-Wall")
>    "A list of extra flags passed to GCC.")
>
> (defvar-local flymake--gcc-cached-flags nil
>    "Internal variable for `flymake-gcc-guess-flags'")
>
> (defun flymake-gcc-guess-flags (&optional trash-cache)
>    "Guess GCC flags for compiling current buffer "
>    (interactive "P")
>    (unless (executable-find "make") (error "Cannot find a suitable make"))
>    (when trash-cache (setq flymake--gcc-cached-flags nil))
>    (catch 'retval
>      (unless (buffer-file-name)
>        ;; don't error and don't cache, so that when the buffer is saved
>        ;; we get another chance.
>        (throw 'retval nil))
>      (when-let* ((makefile-dir
>                   (locate-dominating-file default-directory "Makefile"))
>                  (makefile (expand-file-name "Makefile" makefile-dir))
>                  (mtime (file-attribute-modification-time
>                          (file-attributes makefile))))
>        (cond
>         ((equal (list makefile mtime)
>                 (cdr flymake--gcc-cached-flags))
>          (when (called-interactively-p 'interactive)
>            (message "cached hit for flags for this buffer: %s"
>                     (car flymake--gcc-cached-flags)))
>          (throw 'retval (car flymake--gcc-cached-flags)))
>         (t
>          (let*
>              ((sans-nothing
>                (file-name-nondirectory
>                 (file-name-sans-extension
>                  (buffer-file-name))))
>               (blob (shell-command-to-string
>                      (format "make -C %s -f %s --just-print %s.o"
>                              makefile-dir
>                              makefile
>                              sans-nothing)))
>               (match (string-match
>                       (format "gcc[[:space:]]+\\(\\(?:-.*\\)*\\)%s"
>                               sans-nothing)
>                       blob))
>               (flag-string (and match
>                                 (match-string 1 blob)))
>               (flags (and flag-string
>                           (flymake--gcc-heroic-unescape flag-string))))
>            (when (or flags (string= "" flag-string))
>              (setq flymake--gcc-cached-flags (list flags makefile mtime))
>              (when (called-interactively-p 'interactive)
>                (message "cached miss for flags for this buffer: %s" flags))
>              (throw 'retval flags))))))
>      (error "Could not guess gcc flags")))
>
>
> (defvar-local flymake--gcc-proc nil
>    "Internal variable for `flymake-gcc'")
>
> (defun flymake-gcc (report-fn &rest _args)
>    "Flymake backend for GCC"
>    (unless (executable-find flymake-gcc-program)
>      (error "Cannot find a suitable gcc"))
>    (when (process-live-p flymake--gcc-proc)
>      (kill-process flymake--gcc-proc))
>    (let ((source (current-buffer)))
>      (save-restriction
>        (widen)
>        (setq flymake--gcc-proc
>              (make-process
>               :name "gcc-flymake"
>               :buffer (generate-new-buffer "*gcc-flymake*")
>               :command `(,flymake-gcc-program
>                          "-fsyntax-only"
>                          ,@flymake-gcc-extra-flags
>                          ,@(if (symbolp flymake-gcc-flags)
>                                (funcall flymake-gcc-flags)
>                              flymake-gcc-flags)
>                          "-x" "c" "-")
>               :noquery t :connection-type 'pipe
>               :sentinel
>               (lambda (p _ev)
>                 (when (eq 'exit (process-status p))
>                   (unwind-protect
>                       (when (eq p flymake--gcc-proc)
>                         (with-current-buffer (process-buffer p)
>                           (goto-char (point-min))
>                           (cl-loop
>                            while (search-forward-regexp
>                                   "^<stdin>:\\([0-9]+\\):\\([0-9]+\\): \\(.*\\): \\(.*\\)$"
>                                   nil t)
>                            for msg = (match-string 4)
>                            for (beg . end) =
>                            (flymake-diag-region
>                             source
>                             (string-to-number (match-string 1))
>                             (string-to-number (match-string 2)))
>                            for type = (assoc-default
>                                        (match-string 3)
>                                        '(("error" . :error)
>                                          ("note" . :note)
>                                          ("warning" . :warning))
>                                        #'string-match)
>                            collect (flymake-make-diagnostic source beg end type msg)
>                            into diags
>                            finally (funcall report-fn diags))))
>                     ;; (display-buffer (process-buffer p)) ; use this instead of the next one for debug
>                     (kill-buffer (process-buffer p))
>                     ))
>                 )))
>        (process-send-region flymake--gcc-proc (point-min) (point-max))
>        (process-send-eof flymake--gcc-proc))))
>
> (defun flymake--setup-gcc-flymake ()
>    (add-hook 'flymake-diagnostic-functions 'flymake-gcc nil t))
>
> (add-hook 'c-mode-hook 'flymake--setup-gcc-flymake)
>
> ;;; flymake-gcc.el ends here
>
>
>

Re: New Flymake rewrite in emacs-26

[guillaume papin]

Regarding the way for a project to export it compile options,
there is somewhat popular, if not standard, method.
Generate a compilation database[1].

A compilation database is a JSON file, named compile_commands.json,
with looks like this for emacs:

    [
        {
            "arguments": [
                "cc",
                "-c",
                "-Demacs",
                "-I.",
                "-I../../emacs/src",
                "-I../lib",
                "-I../../emacs/lib",
                "-pthread",
                "-isystem",
                "/usr/include/gtk-3.0",
                <snip...>
                "-isystem",
                "/usr/include/libpng16",
                "-fopenmp",
                "-DMAGICKCORE_HDRI_ENABLE=1",
                "-DMAGICKCORE_QUANTUM_DEPTH=16",
                "-fopenmp",
                "-DMAGICKCORE_HDRI_ENABLE=1",
                "-DMAGICKCORE_QUANTUM_DEPTH=16",
                <snip...>
                "-Wno-unused-parameter",
                "-Wno-format-nonliteral",
                "-g3",
                "-O2",
                "../../emacs/src/intervals.c"
            ],
            "directory": "/home/papin_g/dev/src/emacs/emacs-25.1-build/src",
            "file": "../../emacs/src/intervals.c"
        },
        <snip...>
    ]

I could generate this with a tool called intercept-build[2],
by doing a clean build of emacs:

    $ intercept-build make
    ...
      CC       dbusbind.o
      CC       emacs.o
      CC       keyboard.o
    ...
    $ cat compile_commands.json
        [
            {
                "arguments": [
                    "cc",
                    "-c",
    ...

The compile options have the `-M' filtered out by the tool.

I think there is something in CEDET that knows how to load this information for emacs,
see ede-compdb[2], but this is also quite straightforward to do with the `json' libray.

What the Emacs makefile could do, is generate the JSON file with a command like `make compdb'.
I have done this in the past for project using GNU Make, it wasn't too difficult to implement.

This is also what the cmake-ide tool mentionned just before seems to be using.

[1] https://clang.llvm.org/docs/JSONCompilationDatabase.html
    Sorry for the link to Clang, it's not really Clang-specific.
[2] https://github.com/rizsotto/scan-build
    Once again, related to Clang, sorry about that.
    Bear is basically the same thing, and maybe easier to install from source:
    https://github.com/rizsotto/Bear
[3] https://github.com/randomphrase/ede-compdb
    This is a Github link, but it says that this has been integrated into CEDET.

Re: New Flymake rewrite in emacs-26

[Stefan Monnier]

>            ;; I suspect "shell-command" makes windows even when called
>            ;; from lisp.

Indeed.  Please use `call-process` with `shell-file-name` and
`shell-command-argument` instead.


        Stefan

Re: New Flymake rewrite in emacs-26

[João Távora]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>>            ;; I suspect "shell-command" makes windows even when called
>>            ;; from lisp.
>
> Indeed.  Please use `call-process` with `shell-file-name` and
> `shell-command-argument` instead.

Good to now, but I've scratched the flymake-gcc-heroic-unescape function
for now.  It would be nice to have some portable version of this though:
a function taking a string understood by a certain shell and breaking it
up into arguments suitable for a shellless make-process.

Re: New Flymake rewrite in emacs-26

[João Távora]

guillaume papin <guillaume.papin@epitech.eu> writes:

> Regarding the way for a project to export it compile options,
> there is somewhat popular, if not standard, method.
> Generate a compilation database[1].

This very interesting and obsviously light years ahead of of my naive
attempt, but apparently requires a non-free external tool.

Though hope I've built my patch around some customization points that
could help you deploy it in a third-party extension.

João

Re: New Flymake rewrite in emacs-26

[Stefan Monnier]

> Good to now, but I've scratched the flymake-gcc-heroic-unescape function
> for now.  It would be nice to have some portable version of this though:
> a function taking a string understood by a certain shell and breaking it
> up into arguments suitable for a shellless make-process.

Of course, it's an impossible task in general.
Think of a shell strings such as

    "sed 's/a/b/' <$< | sort -u >$@"
or
    "for f in $</*; do blabla; done"


-- Stefan

Re: New Flymake rewrite in emacs-26

[João Távora]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>> Good to now, but I've scratched the flymake-gcc-heroic-unescape function
>> for now.  It would be nice to have some portable version of this though:
>> a function taking a string understood by a certain shell and breaking it
>> up into arguments suitable for a shellless make-process.
>
> Of course, it's an impossible task in general.

Obsviously, you're right, I didn't think this through, a "string
understood by a shell" is any program written in that shell's language,
and make-process launches a single process.