From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Glenn Morris Newsgroups: gmane.emacs.bugs Subject: bug#17854: The patch #3 of 3 for hideif.el, a lot of bug fixes and enhancements Date: Fri, 27 Jun 2014 21:53:15 -0400 Message-ID: <75lhsh7qb8.fsf@fencepost.gnu.org> References: NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: ger.gmane.org 1403920455 22568 80.91.229.3 (28 Jun 2014 01:54:15 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sat, 28 Jun 2014 01:54:15 +0000 (UTC) Cc: 17854@debbugs.gnu.org To: Luke Lee Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Sat Jun 28 03:54:08 2014 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1X0hqB-0004SN-IN for geb-bug-gnu-emacs@m.gmane.org; Sat, 28 Jun 2014 03:54:07 +0200 Original-Received: from localhost ([::1]:53276 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X0hqB-00042g-8r for geb-bug-gnu-emacs@m.gmane.org; Fri, 27 Jun 2014 21:54:07 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:46358) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X0hq7-00041h-6b for bug-gnu-emacs@gnu.org; Fri, 27 Jun 2014 21:54:04 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1X0hq6-0008Rw-4L for bug-gnu-emacs@gnu.org; Fri, 27 Jun 2014 21:54:03 -0400 Original-Received: from debbugs.gnu.org ([140.186.70.43]:44406) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X0hq6-0008Rs-0y for bug-gnu-emacs@gnu.org; Fri, 27 Jun 2014 21:54:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.80) (envelope-from ) id 1X0hq5-0005DT-Ot for bug-gnu-emacs@gnu.org; Fri, 27 Jun 2014 21:54:01 -0400 X-Loop: help-debbugs@gnu.org In-Reply-To: Resent-From: Glenn Morris Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 28 Jun 2014 01:54:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 17854 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 17854-submit@debbugs.gnu.org id=B17854.140392039819984 (code B ref 17854); Sat, 28 Jun 2014 01:54:01 +0000 Original-Received: (at 17854) by debbugs.gnu.org; 28 Jun 2014 01:53:18 +0000 Original-Received: from localhost ([127.0.0.1]:35556 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1X0hpN-0005CE-Bq for submit@debbugs.gnu.org; Fri, 27 Jun 2014 21:53:17 -0400 Original-Received: from fencepost.gnu.org ([208.118.235.10]:44762 ident=Debian-exim) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1X0hpL-0005C4-N8 for 17854@debbugs.gnu.org; Fri, 27 Jun 2014 21:53:16 -0400 Original-Received: from rgm by fencepost.gnu.org with local (Exim 4.71) (envelope-from ) id 1X0hpL-0007rz-7X; Fri, 27 Jun 2014 21:53:15 -0400 X-Spook: Capricorn CIA investigation Elvis ASO kilo class NWO X-Ran: ";GtZLhjhWYT"8*6(ot-/ig,HT1RH.0]sf3:L;Xe*#CK?>{fJ[A!x#`R#%+1)xKv.SH=nr X-Hue: red X-Attribution: GM User-Agent: Gnus (www.gnus.org), GNU Emacs (www.gnu.org/software/emacs/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 140.186.70.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.bugs:90927 Archived-At: > with white space changes removed (git diff -w) as attached. I hope that the version you install will actually not change the whitespace, rather than that just being hidden with diff -w. > (defcustom hide-ifdef-exclude-define-regexp nil > "Ignore #define names if those names match this exclusion pattern." > - :type 'string) > + :type 'string > + :version "24.4") Why is the :version changing? In any case, it is the wrong :version, since this won't be in 24.4. > +(defcustom hide-ifdef-expand-reinclusion-protection t > + "Prevent hiding the whole C/C++ header file protected by a big #ifdef..#endif. Suggestion: "Non-nil means don't hide an entire header file enclosed by #ifndef...#endif." > +Most C/C++ headers are usually wrapped with ifdefs to prevent re-inclusion: > + > + ----- beginning of file ----- > + #ifdef _XXX_HEADER_FILE_INCLUDED_ #ifndef? > + #define _XXX_HEADER_FILE_INCLUDED_ > + xxx > + xxx > + xxx... > + #endif > + ----- end of file ----- > + > +If we try to hide this header file, for the first time hideif will find > +_XXX_HEADER_FILE_INCLUDED_ and have it defined. Everything between #ifdef > +to #endif are not hidden. Suggestion: "The first time we visit such a file, _XXX_HEADER_FILE_INCLUDED_ is undefined, and so nothing is hidden. The next time we visit it, everything will be hidden." > For the second time since _XXX_HEADER_FILE_INCLUDED_ > +is defined everything between the outermost #ifdef..#endif will be hidden: > + > + ----- beginning of file ----- > + #ifdef _XXX_HEADER_FILE_INCLUDED_ > + ... > + #endif > + ----- end of file ----- I don't think the example is necessary. > +This is not the behavior we expected, we still would like to see the content > +of this header file. With this flag enabled we can have the outermost #if > +always not hidden." Suggestion: "This behavior is generally undesirable. If this option is non-nil, the outermost #if is always visible." (I wonder: why would anyone want to set this option to nil?) > + :type 'boolean > + :version "24.4") 24.5 (Also, new options are the kind of thing to mention in a brief NEWS entry.) > +(defcustom hide-ifdef-header-regexp > + "^.*\\.[hH]\\([hH]\\|[xX][xX]\\|[pP][pP]\\)?" > + "C/C++ header file name patterns to determine if current buffer is a header. > +Effective only if `hide-ifdef-expand-reinclusion-protection' is t." > + :type 'string > + :group 'hide-ifdef) Missing :version. > - current buffer. Initially, the global value of `hide-ifdef-env' > - is used. > + current project. Initially, the global value of `hide-ifdef-env' > + is used. This variable was a buffer-local variable which limits "," before which > + hideif to parse only one C/C++ file at a time. We've extended > + hideif to support parsing a C/C++ project containing multiple C/C++ > + source files opened simultaneously in different buffers. Therefore > + `hide-ifdef-env' can no longer be buffer local but must be global. > + > + We can still simulate the behavior of elder hideif versions (i.e. s/elder/older > + `hide-ifdef-env' being buffer local) by clearing this variable > + (C-c @ C) everytime before hiding current buffer. Does all this detail need to be in the doc-string, or can it just be a comment in the code? (By the way, no need to make ChangeLog entries for comments.) > +(defun hif-clear-all-ifdef-defined () > + "Clears all symbols defined in `hide-ifdef-env'. > +It will backup this variable to `hide-ifdef-env-backup' before clearing to > +prevent accidental clearance." Is that backup really necessary, given that you ask for confirmation? > + (interactive) > + (when (y-or-n-p "Clear all #defined symbols?") > + (setq hide-ifdef-env-backup hide-ifdef-env) > + (setq hide-ifdef-env nil))) > > + ;; Genernally there is no need to call itself recursively since there should Generally > + ((looking-at "\r") ; Sometimes MS-Windows user will leave CR in > + (forward-char 1)) ; the source code. Let's don't stuck here. "Let's not get stuck here." > -(defun hif-possibly-hide () > +(defun hif-possibly-hide (expand-reinclusion) > "Called at #ifX expression, this hides those parts that should be hidden. > It uses the judgment of `hide-ifdef-evaluator'." Doc should say what the argument means. > +(defun hif-evaluate-macro (rstart rend) > + "Evaluate the macro expansion result for a region. > +If no region active, find the current #ifdefs and evaluate the result. Currently Use two spaces between sentences. > +it support only math calculations, strings or argumented macros can not be supports > -(defun hide-ifdef-define (var) > +(defun hide-ifdef-define (var val) Why not make VAL optional? > + "Define a VAR optionally to a specific value VAL into `hide-ifdef-env'. > +This allows #ifdef VAR from being hidden." Suggestion: "Define VAR to VAL (default 1) in `hide-ifdef-env'." > @@ -1618,11 +1860,14 @@ It does not do the work that's pointless to redo on a recursive entry." > Assume that defined symbols have been added to `hide-ifdef-env'. > The text hidden is the text that would not be included by the C > preprocessor if it were given the file with those symbols defined. > +If this command is prefixed, hide also the #ifdefs themselves. Suggestion: "With optional prefix agument ARG, also hide the #ifdefs themselves." > (interactive) > - (message "Hiding...") > + (let ((hide-ifdef-lines current-prefix-arg)) I think this should take an explicit prefix argument. > +(defun hide-ifdef-block (&optional start end) > + "Hide the ifdef block (true or false part) enclosing or before the cursor. > +If prefixed, it will also hide #ifdefs themselves." Suggestion: "With optional prefix agument ARG, also hide the #ifdefs themselves." > + (interactive "r") > + (let ((hide-ifdef-lines current-prefix-arg)) I think this should explicitly take a prefix argument.