Re: [patch] Some fixes for flymake

[Xue Fuqiao <xfq.free@gmail.com>]

On Fri, May 10, 2013 at 9:20 AM, Glenn Morris <rgm@gnu.org> wrote:
>
> Thanks. Looks mostly fine. Some comments follow, otherwise please install.
>
>   -   "Return the corresponding entry from `flymake-allowed-file-name-masks'."
>   +   "Return the function(s) correspond(s) to FILE-NAME in `flymake-allowed-file-name-masks'."
>
> The original is better (the new version is ungrammatical and looks too long).

OK, I'll remove it.  (Although the argument FILE-NAME isn't mentioned in
the doc string, it's not very confusing here.)

>   - (defun flymake-can-syntax-check-file (file-name)
>   -   "Determine whether we can syntax check FILE-NAME.
>   - Return nil if we cannot, non-nil if we can."
>   -   (if (flymake-get-init-function file-name) t nil))
>
>   + (defun flymake-can-syntax-check-file (file-name)
>   +   "Determine whether we can syntax check FILE-NAME.
>   + Return t if we can, nil if we cannot."
>   +   (if (flymake-get-init-function file-name) t nil))
>
> I think the original is better, because the result seems to only ever be
> used in a Boolean sense.

Well, OK.

>     (defun flymake-find-possible-master-files (file-name master-file-dirs masks)
>   -   "Find (by name and location) all possible master files.
>   +   "Find (by FILE-NAME and MASTER-FILE-DIRS) all possible master files.
>
> I prefer the original. It may not slavishly follow what checkdoc says,
> but it is clearer. Add a separate explanation of what the arguments are
> if you want to.
>
>     (defun flymake-check-include (source-file-name inc-name include-dirs)
>   -   "Check if SOURCE-FILE-NAME can be found in include path.
>   +   "Check if SOURCE-FILE-NAME can be found in INCLUDE-DIRS.
>
> As above.

I'll add separate explanations.

>     (defun flymake-save-buffer-in-file (file-name)
>   +   "Save the entire buffer contents into file FILE-NAME.
>   + It also creates optionally any nonexistent parent directories."
>
> Better would be something like: "Create parent directories as needed."
>
>   + ;;  LocalWords:  DIRS POS odl tex init GNUmakefile Makefile
>
> I don't think we want to add LocalWords to files? Not sure...

Sorry, it's my fault.  I was trying to supress errors emitted by
Flyspell Prog mode, and I forgot to delete this section.

> The texi patch seems mainly (entirely?) to be changing from one space
> between sentences to two. That's fine. If there was any change apart
> from that, please point it out separately, otherwise please install.

Right.

--
Best regards, Xue Fuqiao.
http://www.gnu.org/software/emacs/