emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Three Flymake backends Was Re: Two issues with the new Flymake


From: Stefan Monnier
Subject: Re: Three Flymake backends Was Re: Two issues with the new Flymake
Date: Sat, 04 Nov 2017 11:30:25 -0400
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/26.0.50 (gnu/linux)

FWIW, I think we can install those backends into emacs-26, and later
consolidate them in `master`.

> Obviously, this makes maintenance hard if small fixes are made to the
> common structure. The tex-chktex backend recently contributed by Mark is
> already an example of that in that it is missing a fix that this type of
> backend requires (I pushed a fix in the meantime).
>
> So I'm thinking that, for master (_not_ emacs-26) we could use a
> declarative flymake-define-simple-backend macro.

FWIW, I think that "makes maintenance hard if small fixes are made to the
common structure" is also an argument against the use of a macro (or
at least against putting the common structure in the macro's expansion),
since it would imply that you need to recompile the backend in order to
benefit from fixes in the common structure.

Also using a macro like you did makes it difficult to debug/edebug the
common code.  Better move that common code into a function.

See additional comments about the code below,


        Stefan


>   (defvar-local flymake--simple-backend-procs
>     (make-hash-table))

You never `setq` this var, so it will behave like a global var.
More specifically, a single hash-table is used for all buffers, which
I believe is not what you intended.

>   (defmacro flymake-define-simple-backend (name command pattern &optional 
> type-predicate)
>     "Define a simple Flymake backend under NAME.

I had to look at the code to understand what you meant by "under NAME".
I'd have written it more like "Define Flymake backend function named
NAME", maybe.

>   PATTERN must evaluate to a list of the form (REGEXP LINE COLUMN
>   TYPE MESSAGE): if REGEXP matches, the LINE'th subexpression gives
>   the line number, the COLUMN'th subexpression gives the column
>   number on that line, the TYPE'th subexpression gives the type of
>   the message and the MESSAGE'th gives the message text itself.

Line numbers are reasonably standardized, but column numbers aren't.
Some tools start counting from 0 others from 1, some count bytes, others
count chars, yet others count actual "visual" columns.  `compile.el` has
added some vars to control this, but it's rather messy.  So we should
make it possible to provide code which does the conversion to whatever
we use.

Maybe it would also make sense to try and support message which include
both the BEG and the END.

Rather than `type-predicate`, I'd rather just allow `type` to be a function.
Also it'd probably make sense to allow the type returned by that
function to be nil in which case we should just skip the match
(i.e. not create a diagnostic for it).

So I guess I'm looking at something that look like
a `flymake-simple-backend` with signature:

   (flymake-simple-backend COMMAND REGEXP MSG TYPE START &optional END)

where MSG and TYPE are normally integers, and START and END can be
either an integer (line-number submatch) or a list of 2 integers (LINE
COL), and all 4 of those could be functions instead.

>     (let ((name-once name))

I must say I don't get what this <foo>-once business is about.
What is `-once` supposed to mean?  What did you need it for?

> +  (add-hook 'flymake-diagnostic-functions 'perl-flymake nil t))
                                             ^^
                                             #'

-- Stefan




reply via email to

[Prev in Thread] Current Thread [Next in Thread]