* Vincent Belaïche (2010-06-21) writes: > I can split this into 3 differents/subsequent contributions with > respective ChangeLog -- or not, it is up to your feedback. > > If you want 3 separate patches, then could you please tell me in which > order I should provide them.
The order is not really important. The idea is that once we finished discussing one of the patches, I can immediately check it in. We don't have to wait until all of them are discussed. And it obviously makes the code review easier. With the current set of patches we are as good as through, I think. See my remarks below. So it is not that important anymore to split them. > 2010-06-03 Vincent Belaïche <[email protected]> It would be good if we'd only use one email address for your ChangeLog entries. The last one got the Gmail address. > + (defconst Texinfo-structuring-command-re > + (concat "^\\s-*@" > + (funcall 'regexp-opt (mapcar 'car texinfo-section-list) > + 'word)) `regexp-opt' only knows `words', not `word'. Also, is the performance really so bad that the regexp has to be computed here? In latex.el this is done one the fly which has the advantage that you don't have to reset the "variable" when you change `texinfo-section-list'. > + (defun Texinfo-mark-section (&optional to-be-marked) > + "Mark current section, with inclusion of any containing to-be-marked, > + where current section is started by any of the structuring > + commands matched by regexp in constant > + `Texinfo-structuring-command-re'. This docstring is not checkdoc-compliant. It should probably also mention `texinfo-section-list' instead of `Texinfo-structuring-command-re'. > + If optional argument TO-BE-MARKED is set to 1 or is a non nil empty > + argument, then mark current node. > + > + If optional argument TO-BE-MARKED is set to 2 or to `-', then > + mark current section, with exclusion of any subsections." Hm, that interface seems a bit awkard. I'm not sure it is very lispy to use numeric values with the prefix argument like that. The behavior also differs from `LaTeX-mark-section'. Perhaps there should be two separate commands, one for nodes and one for sectioning? With respect to the rest of the code, there are a few lines which are longer than 80 characters and should be shortened. > + (define-key map "\C-c." 'Texinfo-mark-environment) > + (define-key map "\C-c*" 'Texinfo-mark-section) Those bindings are already marked as being dubious in latex.el because the Emacs key binding conventions do not really encourage to use them. So perhaps we should not introduce them in Texinfo mode as well. I'm not sure if there are better alternatives. Lisp mode, for example, has `C-M-h' for `mark-defun'. (Does anybody know where this `h' mnemonic stems from?) But then we are still one binding short. -- Ralf _______________________________________________ auctex-devel mailing list [email protected] http://lists.gnu.org/mailman/listinfo/auctex-devel
