Some stylistic comments only:
It needs a ChangeLog entry, and probably a NEWS entry.
> -;; Daniel LaLiberte <liberte@holonexus.org>
> +;; Daniel LaLiberte <liberte@holonexus.org>
Please don't change existing whitespace in areas that you are not
otherwise touching.
> -;; (unless hide-ifdef-define-alist
> -;; (setq hide-ifdef-define-alist
> -;; '((list1 ONE TWO)
> -;; (list2 TWO THREE))))
> -;; (hide-ifdef-use-define-alist 'list2))) ; use list2 by default
> +;; (unless hide-ifdef-define-alist
> +;; (setq hide-ifdef-define-alist
> +;; '((list1 ONE TWO)
> +;; (list2 TWO THREE))))
> +;; (hide-ifdef-use-define-alist 'list2))) ; use list2 by default
Again, this is just whitespace.
> @@ -129,16 +129,44 @@
> "Non-nil means shadow text instead of hiding it."
> :type 'boolean
> :group 'hide-ifdef
> - :version "23.1")
> + :version "24.5")
>
> (defface hide-ifdef-shadow '((t (:inherit shadow)))
> "Face for shadowing ifdef blocks."
> :group 'hide-ifdef
> - :version "23.1")
> + :version "24.5")
Why is the :version changing, when the defaults are unchanged?
> (defcustom hide-ifdef-exclude-define-regexp nil
> "Ignore #define names if those names match this exclusion pattern."
> :type 'string)
> +(defcustom hide-ifdef-expand-reinclusion-protection t
> + "When hiding header files, enabling this flag allows hideif always try to
> +expand the re-inclusion protected ifdefs. Disabling this flag those headers
> +are usually hidden to a top level #ifdef...#endif due to those defined symbols
The first line of a doc-string should be a complete sentence that fits
in < 80 columns. All the doc should fit within the standard fill-column.
> + :type 'boolean
> + :group 'hide-ifdef)
> +
> +(defcustom hide-ifdef-header-regexp-pattern
> + "^.*\\.[hH]\\([hH]\\|[xX][xX]\\|[pP][pP]\\)?"
> + "C/C++ header file name patterns. Effective only if
> +`hide-ifdef-expand-reinclusion-protection' is t."
> + :type 'string
> + :group 'hide-ifdef)
Again, the first line of the doc should be a complete sentence.
New defcustoms need :version tags (and probably NEWS entries).
> +(defvar hide-ifdef-env-backup nil
> + "A backup variable to prevent `hide-ifdef-env' accidentally cleared by
> +`hif-clear-all-ifdef-defined'.")
First line of doc too long. Also, this is ungrammatical.
> `hide-ifdef-env'
> - An association list of defined and undefined symbols for the
> - current buffer. Initially, the global value of `hide-ifdef-env'
> - is used.
> + An association list of defined and undefined symbols for the
> + current project. Initially, the global value of `hide-ifdef-env'
> + is used. This variable was a buffer-local variable but is now a
> + global variable since we've extend hideif to support project-based
s/extend/extended.
"project-based across all-buffers" doesn't make sense.
I'm not sure that describing how things used to work is helpful.
> + across all-buffers. To simulate the original buffer local behavior
> + we need to clear this variable (C-c @ C) then hide current buffer.
> `hide-ifdef-define-alist'
> - An association list of defined symbol lists.
> + An association list of defined symbol lists.
whitespace.
> - Set to non-nil to not show #if, #ifdef, #ifndef, #else, and
> - #endif lines when hiding.
> + Set to non-nil to not show #if, #ifdef, #ifndef, #else, and
> + #endif lines when hiding.
whitespace.
At this point, I'll give up, and ask you to send a version that does not
have pointless whitespace changes. :)