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: Wed, 07 Dec 2011 11:10:23 -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> NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: dough.gmane.org 1323274238 8965 80.91.229.12 (7 Dec 2011 16:10:38 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Wed, 7 Dec 2011 16:10:38 +0000 (UTC) To: emacs-devel@gnu.org Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Wed Dec 07 17:10:33 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 1RYK4n-0004yv-6P for ged-emacs-devel@m.gmane.org; Wed, 07 Dec 2011 17:10:33 +0100 Original-Received: from localhost ([::1]:44185 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RYK4m-00019A-IU for ged-emacs-devel@m.gmane.org; Wed, 07 Dec 2011 11:10:32 -0500 Original-Received: from eggs.gnu.org ([140.186.70.92]:38558) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RYK4j-00018z-Cz for emacs-devel@gnu.org; Wed, 07 Dec 2011 11:10:30 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RYK4f-0003iu-8q for emacs-devel@gnu.org; Wed, 07 Dec 2011 11:10:29 -0500 Original-Received: from chene.dit.umontreal.ca ([132.204.246.20]:32788) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RYK4f-0003ij-4c for emacs-devel@gnu.org; Wed, 07 Dec 2011 11:10:25 -0500 Original-Received: from faina.iro.umontreal.ca (lechon.iro.umontreal.ca [132.204.27.242]) by chene.dit.umontreal.ca (8.14.1/8.14.1) with ESMTP id pB7GANXk031165; Wed, 7 Dec 2011 11:10:24 -0500 Original-Received: by faina.iro.umontreal.ca (Postfix, from userid 20848) id D718AB42C1; Wed, 7 Dec 2011 11:10:23 -0500 (EST) In-Reply-To: <8739cwtzh8.fsf@lifelogs.com> (Ted Zlatanov's message of "Wed, 07 Dec 2011 06:04:03 -0500") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.0.92 (gnu/linux) X-NAI-Spam-Flag: NO X-NAI-Spam-Threshold: 5 X-NAI-Spam-Score: 0 X-NAI-Spam-Rules: 1 Rules triggered RV4064=0 X-NAI-Spam-Version: 2.2.0.9309 : core <4064> : streams <708607> : uri <1023459> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 132.204.246.20 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:146554 Archived-At: >>> I'd like to rename `cfengine-mode' to `cfengine2-mode', and >>> `cfengine-auto-mode' to `cfengine-mode' (`cfengine-auto-mode' loads >>> `cfengine3-mode' if it thinks a cfengine v.3 file is edited). SM> That sounds right. > Thanks for looking. Attached is a patch that will: > 1) rename `cfengine-mode' to `cfengine2-mode' and defalias > 'cfengine-mode to 'cfengine-auto-mode. Provide 'cfengine2. That's good. > 2) rename all the "cfengine-*" variables used only by `cfengine2-mode' > to "cfengine2-*" Is this needed? Do these variables only make sense for cfengine2 syntax? > 3) adjust the commentary to match Good. > 4) give a version and set it to 1.1 for this release We don't like versions for files bundled with Emacs (only serves to cause spurious merge conflicts), so if you can keep this outside of the tree, it's preferable. > 5) use "CFEngine" consistently, capitalized as the author, Mark Burgess, > does. Good. > 6) add `cfengine3-mode-verbose' to control some debugging info Why not `cfengine-mode-verbose'? Also, if it's for debugging, why call it "verbose" rather than "debug"? > 7) add `cfengine3-parameters-indent' to address some shortcomings in > indentation. This is a new feature but necessary to fix the lack of a > hanging indent in the current `cfengine3-mode'. It's very low-risk and > I hope it's acceptable. I'd keep it with a "cfengine-" prefix. Basically, I'd keep user-facing names with a "cfengine-" prefix whenever possible. > 8) improve and capitalize comments in many places, though that work is > not complete Good. > 9) change the "lighter" modeline indicators from "Cfengine[23]" to > "CFE[23]" to take less space No opinion on this. > I also have a font-lock issue I can't figure out with the current > version. Given the following code (use `cfengine3-mode' to see the > problem): *Messages* tells me: Error during redisplay: (void-variable font-lock-constant-name-face) Some comments on the code follow. Stefan > +(defvar cfengine-version nil) > +(setq cfengine-version "1.1") Regardless of whether we keep this, the above is atrocious. Use `defconst'. > +(defcustom cfengine3-parameters-indent '(promise pname 0) > + "*Indentation of CFEngine3 promise parameters (hanging indent)." This description is incomprehensible to me. It should describe the accepted values and their meaning. Also better include a short example to make it clear what this controls. > +(defcustom cfengine2-mode-abbrevs nil > + "Abbrevs for CFEngine2 mode." > :group 'cfengine > :type '(repeat (list (string :tag "Name") > (string :tag "Expansion") You haven't added the corresponding feature for cfengine3, and I think I agree with this choice, but the reason is not that it made sense for cfengine2 and it doesn't for cfengine3, but simply that there's no need for a config var for it. There's already `edit-abbrevs'. So rather than rename it, I'd obsolete it and mention that it only applies to cfengine2-mode. > - (defconst cfengine-actions > + (defconst cfengine2-actions I think cfengine-font-lock-keywords also needs to be renamed to cfengine2 prefix. > (defvar cfengine3-font-lock-keywords > `( > - (,(concat "^[ \t]*" cfengine3-class-selector-regex) > - 1 font-lock-keyword-face) > - (,(concat "^[ \t]*" cfengine3-category-regex) > - 1 font-lock-builtin-face) > - ;; Variables, including scope, e.g. module.var > - ("[@$](\\([[:alnum:]_.]+\\))" 1 font-lock-variable-name-face) > - ("[@$]{\\([[:alnum:]_.]+\\)}" 1 font-lock-variable-name-face) > - ;; Variable definitions. > - ("\\<\\([[:alnum:]_]+\\)[ \t]*=[ \t]*(" 1 font-lock-variable-name-face) > - > - ;; CFEngine 3.x faces > ;; defuns > (,(concat "\\<" cfengine3-defuns-regex "\\>" > "[ \t]+\\<\\([[:alnum:]_]+\\)\\>" > @@ -136,27 +144,43 @@ > (2 font-lock-constant-name-face) > (3 font-lock-function-name-face) > (5 font-lock-variable-name-face)) > + > + ;; class selector > + (,(concat "^[ \t]*" cfengine3-class-selector-regex) > + 1 font-lock-keyword-face) > + > + ;; category > + (,(concat "^[ \t]*" cfengine3-category-regex) > + 1 font-lock-builtin-face) > + > + ;; Variables, including scope, e.g. module.var > + ("[@$](\\([[:alnum:]_.]+\\))" 1 font-lock-variable-name-face) > + ("[@$]{\\([[:alnum:]_.]+\\)}" 1 font-lock-variable-name-face) > + > + ;; Variable definitions. > + ("\\<\\([[:alnum:]_]+\\)[ \t]*=[ \t]*(" 1 font-lock-variable-name-face) > + > ;; variable types > (,(concat "\\<" (eval-when-compile (regexp-opt cfengine3-vartypes t)) "\\>") > 1 font-lock-type-face))) Not sure what this is about. You'd need to describe it in the ChangeLog. The comments should be capitalized. > +(provide 'cfengine2) Don't bother. Stefan