emacs-devel
[Top][All Lists]
Advanced

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

Re: [patch] Some fixes for flymake


From: Glenn Morris
Subject: Re: [patch] Some fixes for flymake
Date: Thu, 09 May 2013 21:20:27 -0400
User-agent: Gnus (www.gnus.org), GNU Emacs (www.gnu.org/software/emacs/)

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).
  
  - (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. (I have no idea why this isn't just an alias to
flymake-get-init-function... )

    (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.

    (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...


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.



reply via email to

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