all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Stefan Monnier <monnier@iro.umontreal.ca>
To: emacs-devel@gnu.org
Subject: Re: Three Flymake backends Was Re: Two issues with the new Flymake
Date: Sat, 04 Nov 2017 11:30:25 -0400	[thread overview]
Message-ID: <jwvmv42nlv9.fsf-monnier+gmane.emacs.devel@gnu.org> (raw)
In-Reply-To: 87po8zrubl.fsf@gmail.com

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	other threads:[~2017-11-04 15:30 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-03  9:50 Two issues with the new Flymake Lele Gaifax
2017-11-03 12:33 ` Stefan Monnier
2017-11-03 14:07   ` Lele Gaifax
2017-11-03 16:59     ` João Távora
2017-11-03 17:15       ` Stefan Monnier
2017-11-03 20:17 ` Three Flymake backends Was " João Távora
2017-11-04 15:30   ` Stefan Monnier [this message]
2017-11-04 23:17     ` João Távora
2017-11-05 12:50   ` Dmitry Gutov
2017-11-05 12:59     ` João Távora
2017-11-05 13:04       ` Dmitry Gutov
2017-11-05 13:22         ` João Távora
2017-11-05 20:14           ` Dmitry Gutov
2017-11-05 21:05             ` João Távora
2017-11-05 23:56               ` Dmitry Gutov
2017-11-06  9:48                 ` João Távora
2017-11-06 10:35                   ` Dmitry Gutov
2017-11-06 11:08                     ` João Távora
2017-11-13  0:23                       ` Dmitry Gutov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=jwvmv42nlv9.fsf-monnier+gmane.emacs.devel@gnu.org \
    --to=monnier@iro.umontreal.ca \
    --cc=emacs-devel@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.