bug#28808: [PATCH] Implement Python backend for Flymake

[Lele Gaifax <lele@metapensiero.it>]

Thank you João!

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

>> +(defcustom python-flymake-command '("pyflakes")
>> +  "The external tool that will be used to perform the syntax check.
>> +This is a non empty list of strings, the checker tool possibly followed by
>> +required arguments: to use `flake8' you would set this to (\"flake8\" \"-\")."
>>
> I wonder if you shouldn't mention here that the command produced should,
> once invoked, check (a file? a chunk?) of python source code passed to
> it via its standard input.

Ok, done.

>> +(defcustom python-flymake-command-output-regexp
>> +  "^\\(?:<stdin>\\):\\(?1:[0-9]+\\):\\(?:\\(?2:[0-9]+\\):\\)? \\(?3:.*\\)$"
>> +  "The regexp used to parse the output of the specified tool.
>> +It must contain two or three groups: group 1 is the line number, group 2 the
>> +optional column number and the third is the actual message."
>
> A common trick here that old flymake (and also compile.el) use is to
> define the variable's value as list (REGEXP LINE COLUMN TYPE
> MESSAGE).

I will try to better understand this, as I failed to see the benefit of adding
that indirection... maybe the compile.el functionality is older than the
ability to use explicit group numbers in the regexp?

>> +  (unless (derived-mode-p 'python-mode)
>> +    (error "Can only work on `python-mode' buffers"))
>
> Stefan and I arrived at the conclusion that this is cruft and isn't
> needed. 

Ok, removed.

>> +(defun python-flymake-activate ()
>
> Rename this to python--flymake-setup, because "activation" is actually
> enabling flymake-mode.

Ok, done.

> Also, I think you should add an autoload cookie
> to python--flymake-setup and then call that function from the end of
> python-mode. The

I'm curious here: why the need of autoload cookie, if the (only?) caller site
lives in the very same source file? And if that's not only caller, why marking
it private with the double dash?

More importantly: if we unconditionally activate the Flymake feature, instead
of being an user's choice, then the python--flymake-setup function may go
away, and the add-hook moved inside the python-mode function, it already
contains lot of those...

>> +  "Activate the Flymake syntax check on all python-mode buffers."
>> +  (add-hook 'flymake-diagnostic-functions #'python-flymake nil t))
>
> I'd use 'python-flymake instead of #'python-flymake in add-hook, but I
> can't offer a sound reason why :-)

Well, my habit is different here, and it's also the convention used by the
rest of python.el, so I will leave that as is.

See http://endlessparentheses.com/get-in-the-habit-of-using-sharp-quote.html
for what I considered a "sound reason" :)

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.