[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Fwd: Update earlier posted hideif.el enhancements
From: |
Stefan Monnier |
Subject: |
Re: Fwd: Update earlier posted hideif.el enhancements |
Date: |
Fri, 29 Nov 2013 16:59:55 -0500 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3.50 (gnu/linux) |
> Sure, no problem. Attached is the patch against Emacs trunk.
Thanks. Came through unadulterated ;-)
> I'm using git://git.savannah.gnu.org/emacs.git as my Emacs remote
> source. I've checked the trunk, master and emacs-24 and they seems to
> have the identical hideif.el version so I think it should be okay to
> patch it.
Indeed, there's been no real development on it in recently. See my
comments below. Overall, I think it looks good and could be included in
hideif.el, but I'd appreciate if you could clean up a few things first
and split the change into a few chunks (this is a bit large to review as
one single chunk). IIUC one chunk would focus on extend the current
parser & evaluator to handle the full CPP syntax.
Stefan
> (if hide-ifdef-mode
> (progn
> ;; inherit global values
> - (set (make-local-variable 'hide-ifdef-env)
> - (default-value 'hide-ifdef-env))
> +;; (set (make-local-variable 'hide-ifdef-env) ;; [2012-09-27 11:05:26
> +0800] Luke rem 2 ins 2
> +;; (default-value 'hide-ifdef-env))
> + (set 'hide-ifdef-env (default-value 'hide-ifdef-env))
> (set (make-local-variable 'hide-ifdef-hiding)
> (default-value 'hide-ifdef-hiding))
> (set (make-local-variable 'hif-outside-read-only) buffer-read-only)
I don't undertand this change. "(set '<foo> ...)" is just a bad form of
(setq <foo> ...) so use `setq' instead.
> +(defun merge-ifdef-region (start end)
This lacks the "hif-" or "hide-ifdef-" prefix.
> ;;;-----------------------------------------------------------------
> ;;; Translate C preprocessor #if expressions using recursive descent.
> ;;; This parser is limited to the operators &&, ||, !, and "defined".
> ;;; Added ==, !=, +, and -. Gary Oberbrunner, address@hidden, 8/9/94
> +;;;
> +;;; Implement the C language operator precedence table. Add all those
> +;;; missing operators that could be used in macros. Luke Lee 2013-09-04
";;;" should be limited to headers. The pre-existing code is already at
fault, but we should fix the pre-existing code rather than repeating
its mistake.
> +;;; | Unary Operators | * & + - ! ~ ++expr --expr (typecast) sizeof |
> right-to-left |
Try really hard to stay within 80 columns, please.
> +(defun hif-expand-token-list (tokens &optional macroname reflist) ;;
> reference list to prevent self-referencing
Same here. Also, we like to capitalize our comments and terminate them
with full-stops.
BTW, maybe we could/should add some code to smie.el so as to not only
skip a expression but return a parse tree. Then we could use maybe it here.
> +(defun shiftleft (a b)
> +(defun shiftright (a b)
> +(defun flatten (l)
> +(defun delimit (l a)
Lack the "hif-" prefix.