From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.devel Subject: Re: changes to cfengine-mode Date: Fri, 16 Dec 2011 19:45:53 -0500 Message-ID: References: <87ipmcgjot.fsf@lifelogs.com> <83lir89gne.fsf@gnu.org> <87aa7ogfo3.fsf@lifelogs.com> <87fwhffv37.fsf@lifelogs.com> <877h2ridev.fsf@gnu.org> <8739dfezc1.fsf@lifelogs.com> <87aa7m5a6b.fsf@gnu.org> <877h2oe3yj.fsf@lifelogs.com> <877h2ngnhb.fsf@marauder.physik.uni-ulm.de> <87hb1qb9ot.fsf@lifelogs.com> <87pqg82tdy.fsf@lifelogs.com> <87pqg7x89t.fsf_-_@lifelogs.com> <8739cwtzh8.fsf@lifelogs.com> <87iplk9ylb.fsf@lifelogs.com> <87pqfs16x0.fsf@lifelogs.com> NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: dough.gmane.org 1324082766 5511 80.91.229.12 (17 Dec 2011 00:46:06 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Sat, 17 Dec 2011 00:46:06 +0000 (UTC) To: emacs-devel@gnu.org Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sat Dec 17 01:46:03 2011 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([140.186.70.17]) by lo.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1RbiPa-0003Xi-CJ for ged-emacs-devel@m.gmane.org; Sat, 17 Dec 2011 01:46:02 +0100 Original-Received: from localhost ([::1]:58569 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RbiPZ-0000kK-Ei for ged-emacs-devel@m.gmane.org; Fri, 16 Dec 2011 19:46:01 -0500 Original-Received: from eggs.gnu.org ([140.186.70.92]:35650) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RbiPW-0000kF-9M for emacs-devel@gnu.org; Fri, 16 Dec 2011 19:45:59 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RbiPV-0000A7-43 for emacs-devel@gnu.org; Fri, 16 Dec 2011 19:45:58 -0500 Original-Received: from smtp-04.vtx.ch ([194.38.175.93]:54586) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RbiPU-0000A3-OT for emacs-devel@gnu.org; Fri, 16 Dec 2011 19:45:57 -0500 Original-Received: from ceviche.home (dyn.83-228-219-171.dsl.vtx.ch [83.228.219.171]) by smtp-04.vtx.ch (VTX Services SA) with ESMTP id D376C29B499; Sat, 17 Dec 2011 01:45:54 +0100 (CET) Original-Received: by ceviche.home (Postfix, from userid 20848) id 515F1662EE; Fri, 16 Dec 2011 19:45:53 -0500 (EST) In-Reply-To: <87pqfs16x0.fsf@lifelogs.com> (Ted Zlatanov's message of "Tue, 13 Dec 2011 15:46:35 -0600") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.0.92 (gnu/linux) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6, seldom 2.4 (older, 4) X-Received-From: 194.38.175.93 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:146759 Archived-At: > OK. See attached, and I hope final, patch. A few more nit picks, feel free to install the patch after addressing them. > +(defcustom cfengine-mode-debug nil > + "Whether `cfengine-mode' should print debugging info > + (only used by the cfengine3 support currently)" > + :group 'cfengine > + :type 'boolean) We usually make those debug variables with `defvar' rather than `defcustom'. > (defcustom cfengine-mode-abbrevs nil > - "Abbrevs for Cfengine mode." > + "Abbrevs for CFEngine2 mode. > +Obsolete, see `edit-abbrevs' instead." [...] > +(make-obsolete-variable 'cfengine-mode-abbrevs nil "24.1") Don't mention "Obsolete" in the docstring, since the make-obsolete-variable already gives this info. And please put the "use edit-abbrevs" as second argument to make-obsolete-variable instead. > (defvar cfengine3-font-lock-keywords > `( > + ;; defuns Please capitalize your comments. > + (,(concat "\\<" cfengine3-defuns-regex "\\>" > + "[ \t]+\\<\\([[:alnum:]_]+\\)\\>" > + "[ \t]+\\<\\([[:alnum:]_]+\\)\\((\\([^)]*\\))\\)") > + (1 font-lock-builtin-face) > + (2 font-lock-constant-face) > + (3 font-lock-function-name-face) > + (5 font-lock-variable-name-face)) > + > + (,(concat "\\<" cfengine3-defuns-regex "\\>" > + "[ \t]+\\<\\([[:alnum:]_]+\\)\\>" > + "[ \t]+\\<\\([[:alnum:]_]+\\)") > + (1 font-lock-builtin-face) > + (2 font-lock-constant-face) > + (3 font-lock-function-name-face)) Why not merge the two entries into something like: (,(concat "\\<" cfengine3-defuns-regex "\\>" "[ \t]+\\<\\([[:alnum:]_]+\\)\\>" "[ \t]+\\<\\([[:alnum:]_]+\\)\\(?:(\\([^)]*\\))\\)?") (1 font-lock-builtin-face) (2 font-lock-constant-face) (3 font-lock-function-name-face) (4 font-lock-variable-name-face nil t)) And please make sure you explain in the commit log (or here in comments) why this code was moved to the beginning of cfengine3-font-lock-keywords. Stefan