Re: [AUCTeX] Patch to tex-info.el to support node name completion in @..ref commands, and solve a few other pbs
Hi Vincent, 2015-11-11 15:32 GMT+01:00 Vincent Belaïche : > Ooops... I had completely forgotten about that. At least that demonstrate how > constant I am in my way of thinking. > So, the only thing to do is to replace the assoc-string by this > TeX-assoc-string, right ? Right, I've already done that ;-) Cheers, Mosè ___ auctex mailing list auctex@gnu.org https://lists.gnu.org/mailman/listinfo/auctex
Re: [AUCTeX] Patch to tex-info.el to support node name completion in @..ref commands, and solve a few other pbs
Ooops... I had completely forgotten about that. At least that demonstrate how constant I am in my way of thinking. So, the only thing to do is to replace the assoc-string by this TeX-assoc-string, right ? Vincent. > From: m...@gnu.org > Date: Mon, 9 Nov 2015 22:48:05 +0100 > Subject: Re: [AUCTeX] Patch to tex-info.el to support node name completion in > @..ref commands, and solve a few other pbs > To: vincent@hotmail.fr > CC: auctex@gnu.org > > Hi Vincent, > > 2015-11-09 17:55 GMT+01:00 Vincent Belaïche : >> Dear Mosé, >> >> No there was no strong reason for assoc-string rather than assoc, the >> point was just that when you know that the key is a string assoc-string >> is supposed to be faster. >> >> My opinion --- with no regard to this specific patch --- is that it >> would better in the case of XEmacs to define assoc-string as an alias of >> assoc if the function is unbound. > > Well, it turned out there was already an alias `TeX-assoc-string' you > defined last year ;-) > > Bye, > Mosè ___ auctex mailing list auctex@gnu.org https://lists.gnu.org/mailman/listinfo/auctex
Re: [AUCTeX] Patch to tex-info.el to support node name completion in @..ref commands, and solve a few other pbs
Hi Vincent, 2015-11-09 17:55 GMT+01:00 Vincent Belaïche : > Dear Mosé, > > No there was no strong reason for assoc-string rather than assoc, the > point was just that when you know that the key is a string assoc-string > is supposed to be faster. > > My opinion --- with no regard to this specific patch --- is that it > would better in the case of XEmacs to define assoc-string as an alias of > assoc if the function is unbound. Well, it turned out there was already an alias `TeX-assoc-string' you defined last year ;-) Bye, Mosè ___ auctex mailing list auctex@gnu.org https://lists.gnu.org/mailman/listinfo/auctex
Re: [AUCTeX] Patch to tex-info.el to support node name completion in @..ref commands, and solve a few other pbs
Dear Mosé, No there was no strong reason for assoc-string rather than assoc, the point was just that when you know that the key is a string assoc-string is supposed to be faster. My opinion --- with no regard to this specific patch --- is that it would better in the case of XEmacs to define assoc-string as an alias of assoc if the function is unbound. Furthermore, I must say that the way I wrote this patch is quite generic because I have anticipated that there are other specials characters (hashsign, backslash, etc...) that we may want some day to de-escape. However currently the map contains only the comma character, so using assoc[-string] to search a map of one element might be a little over-engineered. The code could be simplified without use of a map if everybody is content with not anticipating any other escapable characters. VBR, Vincent > From: m...@gnu.org > Date: Sun, 8 Nov 2015 01:06:26 +0100 > Subject: Re: [AUCTeX] Patch to tex-info.el to support node name completion in > @..ref commands, and solve a few other pbs > To: vincent@hotmail.fr > CC: auctex@gnu.org > > Hi Vincent, > > 2015-10-25 21:15 GMT+01:00 Vincent Belaïche : >> Dear AUCTeX experts, >> >> I would like to submit to you the attached patch. >> >> [...] > > XEmacs doesn't have `assoc-string', is it possible to use `assoc' or > there was a strong reason for using `assoc-string' instead? > > Bye, > Mosè ___ auctex mailing list auctex@gnu.org https://lists.gnu.org/mailman/listinfo/auctex
Re: [AUCTeX] Patch to tex-info.el to support node name completion in @..ref commands, and solve a few other pbs
Hi Vincent, 2015-10-25 21:15 GMT+01:00 Vincent Belaïche : > Dear AUCTeX experts, > > I would like to submit to you the attached patch. > > [...] XEmacs doesn't have `assoc-string', is it possible to use `assoc' or there was a strong reason for using `assoc-string' instead? Bye, Mosè ___ auctex mailing list auctex@gnu.org https://lists.gnu.org/mailman/listinfo/auctex
Re: [AUCTeX] Patch to tex-info.el to support node name completion in @..ref commands, and solve a few other pbs
Vincent Belaïche writes: >>> PS: BTW, what is the use of calling `nreverse' over `nodes' at >>> function return ? >> >> Just to have the list of nodes be in the definition order. That's not >> really required but more intuitive. > > The list is only used in a `completing-read' AFAIK, so the entries are > presented to user in alphabetical order whatever the order of the > list. That's true if `completing-read-function' is `completing-read-default' but user's are free to use a different function. For example, I use ivy [1] which will usually present the candidates in order though you can define different sortings, too, e.g., with C-h v/f you get alphabetical sorting, with M-x you can have sorting by frequency, etc. Bye, Tassilo ___ auctex mailing list auctex@gnu.org https://lists.gnu.org/mailman/listinfo/auctex
Re: [AUCTeX] Patch to tex-info.el to support node name completion in @..ref commands, and solve a few other pbs
Answers embedded below...
> From: t...@gnu.org
> To: vincent@hotmail.fr
> CC: auctex@gnu.org
> Subject: Re: [AUCTeX] Patch to tex-info.el to support node name completion in
> @..ref commands, and solve a few other pbs
> Date: Wed, 28 Oct 2015 10:32:42 +0100
>
> Vincent Belaïche writes:
>
>> I have not tested the change, but reading this:
[...]
>> are to do more clever things it would be better to integrate the
>> change with reftex so that node references can also appear in the
>> reftex *toc* buffer and we also get all the multifile stuff. Anyway
>> the current code does not handle the @anchor{...} reference, just
>> nodes --- better than nothing.
>
> Patches welcome!
>
Some day yes, ... that is something I have wanted to do since the time
when I added the *toc* support to Texinfo.
>> PS: BTW, what is the use of calling `nreverse' over `nodes' at
>> function return ?
>
> Just to have the list of nodes be in the definition order. That's not
> really required but more intuitive.
>
The list is only used in a `completing-read' AFAIK, so the entries are
presented to user in alphabetical order whatever the order of the list.
> Bye,
> Tassilo
Bye,
Vincent.
___
auctex mailing list
auctex@gnu.org
https://lists.gnu.org/mailman/listinfo/auctex
Re: [AUCTeX] Patch to tex-info.el to support node name completion in @..ref commands, and solve a few other pbs
Mosè Giordano writes: >> Ok, now I've fixed it and use `display-warning' with CLASS set to >> 'AUCTeX so that users also see who issued the warning. That's much >> better. But I'm not entirely sure if very old emacsen have that: I >> checked GNU Emacs 22+ and XEmacs 21.5 but it would be great if >> somebody could check Emacs 21.4 and XEmacs 21.4. > > Didn't test the whole patch, but I can confirm XEmacs 21.4 has > display-warning. Perfect, thank you. Bye, Tassilo ___ auctex mailing list auctex@gnu.org https://lists.gnu.org/mailman/listinfo/auctex
Re: [AUCTeX] Patch to tex-info.el to support node name completion in @..ref commands, and solve a few other pbs
Il 28/ott/2015 10:33, "Tassilo Horn" ha scritto: > > Vincent Belaïche writes: > > > I have not tested the change, but reading this: > > > > (message "There are duplicate nodes:") > > (dolist (dup (nreverse dups)) > > (message "%s on line %d" (car dup) (cdr dup > > > > I gather that the "There are duplicate nodes:" message will be output > > even when dups is nil. Furthermore if you have N duplicates you will have > > N+1 message and only the last one will remain in the minibuffer --- > > meaning that the user has to go to the *Message* buffer. That is why I > > was using the `warn' function. If you want to use `message' it might be > > better to contatenate all the messages together, maybe something like > > this (not tested): > > > > (and dups > >(message "There are duplicate nodes: %s" > > (mapconcat (lambda (x) (format "%s on line %d" > >(car x) (cdr x))) (nreverse dups) "\n\t"))) > > Gosh, and there was also a void-variable error because I didn't test > again after removing the nodes argument. ;-) > > Ok, now I've fixed it and use `display-warning' with CLASS set to > 'AUCTeX so that users also see who issued the warning. That's much > better. But I'm not entirely sure if very old emacsen have that: I > checked GNU Emacs 22+ and XEmacs 21.5 but it would be great if somebody > could check Emacs 21.4 and XEmacs 21.4. Didn't test the whole patch, but I can confirm XEmacs 21.4 has display-warning. Bye, Mosè ___ auctex mailing list auctex@gnu.org https://lists.gnu.org/mailman/listinfo/auctex
Re: [AUCTeX] Patch to tex-info.el to support node name completion in @..ref commands, and solve a few other pbs
Vincent Belaïche writes:
> I have not tested the change, but reading this:
>
> (message "There are duplicate nodes:")
> (dolist (dup (nreverse dups))
> (message " %s on line %d" (car dup) (cdr dup
>
> I gather that the "There are duplicate nodes:" message will be output
> even when dups is nil. Furthermore if you have N duplicates you will have
> N+1 message and only the last one will remain in the minibuffer ---
> meaning that the user has to go to the *Message* buffer. That is why I
> was using the `warn' function. If you want to use `message' it might be
> better to contatenate all the messages together, maybe something like
> this (not tested):
>
> (and dups
> (message "There are duplicate nodes: %s"
> (mapconcat (lambda (x) (format "%s on line %d"
> (car x) (cdr x))) (nreverse dups) "\n\t")))
Gosh, and there was also a void-variable error because I didn't test
again after removing the nodes argument. ;-)
Ok, now I've fixed it and use `display-warning' with CLASS set to
'AUCTeX so that users also see who issued the warning. That's much
better. But I'm not entirely sure if very old emacsen have that: I
checked GNU Emacs 22+ and XEmacs 21.5 but it would be great if somebody
could check Emacs 21.4 and XEmacs 21.4.
>> I've also removed the optional NODES parameter because apparently nobody
>> used it anyway.
>
> Yes, I had used this nodes argument in case that the code is evolved
> to do partial rescan instead of full rescan... But I agree that this
> is useless for now, let us not make the code confusing. Anyway, if we
> are to do more clever things it would be better to integrate the
> change with reftex so that node references can also appear in the
> reftex *toc* buffer and we also get all the multifile stuff. Anyway
> the current code does not handle the @anchor{...} reference, just
> nodes --- better than nothing.
Patches welcome!
> PS: BTW, what is the use of calling `nreverse' over `nodes' at
> function return ?
Just to have the list of nodes be in the definition order. That's not
really required but more intuitive.
Bye,
Tassilo
___
auctex mailing list
auctex@gnu.org
https://lists.gnu.org/mailman/listinfo/auctex
Re: [AUCTeX] Patch to tex-info.el to support node name completion in @..ref commands, and solve a few other pbs
> From: t...@gnu.org
> To: vincent@hotmail.fr
> CC: auctex@gnu.org
> Subject: Re: [AUCTeX] Patch to tex-info.el to support node name completion in
> @..ref commands, and solve a few other pbs
> Date: Wed, 28 Oct 2015 08:43:08 +0100
>
> Vincent Belaïche writes:
>
>>> If there's something else I have to do, please tell me.
>>
>> Indeed, this revision which I had proposed seems to be useless as far
>> as execution speed is concerned --- at least that was not why Stefan
>> did the change, and nobody had complained.
>>
>> However it still have some added value in terms of warning the user in
>> case that a duplicate node name is detected.
>
> Ah, right, that's indeed useful.
>
>> So, I am not really opinionated whether this further change is needed,
>> it is up to you...
>
> I think the hash-table approach is a bit over-engineered. I've
> committed something simpler in
>
> http://git.savannah.gnu.org/cgit/auctex.git/commit/?id=fb09c26aa539c1a3a48ccf78098ed536f48c4870
>
I have not tested the change, but reading this:
(message "There are duplicate nodes:")
(dolist (dup (nreverse dups))
(message " %s on line %d" (car dup) (cdr dup
I gather that the "There are duplicate nodes:" message will be output
even when dups is nil. Furthermore if you have N duplicates you will have
N+1 message and only the last one will remain in the minibuffer ---
meaning that the user has to go to the *Message* buffer. That is why I
was using the `warn' function. If you want to use `message' it might be
better to contatenate all the messages together, maybe something like
this (not tested):
(and dups
(message "There are duplicate nodes: %s"
(mapconcat (lambda (x) (format "%s on line %d"
(car x) (cdr x))) (nreverse dups) "\n\t")))
> I've also removed the optional NODES parameter because apparently nobody
> used it anyway.
>
Yes, I had used this nodes argument in case that the code is evolved to
do partial rescan instead of full rescan... But I agree that this is
useless for now, let us not make the code confusing. Anyway, if we are
to do more clever things it would be better to integrate the change with
reftex so that node references can also appear in the reftex *toc* buffer
and we also get all the multifile stuff.
Anyway the current code does not handle the @anchor{...} reference, just
nodes --- better than nothing.
> Bye,
> Tassilo
Bye,
Vincent.
PS: BTW, what is the use of calling `nreverse' over `nodes' at function return ?
___
auctex mailing list
auctex@gnu.org
https://lists.gnu.org/mailman/listinfo/auctex
Re: [AUCTeX] Patch to tex-info.el to support node name completion in @..ref commands, and solve a few other pbs
Vincent Belaïche writes: >> If there's something else I have to do, please tell me. > > Indeed, this revision which I had proposed seems to be useless as far > as execution speed is concerned --- at least that was not why Stefan > did the change, and nobody had complained. > > However it still have some added value in terms of warning the user in > case that a duplicate node name is detected. Ah, right, that's indeed useful. > So, I am not really opinionated whether this further change is needed, > it is up to you... I think the hash-table approach is a bit over-engineered. I've committed something simpler in http://git.savannah.gnu.org/cgit/auctex.git/commit/?id=fb09c26aa539c1a3a48ccf78098ed536f48c4870 I've also removed the optional NODES parameter because apparently nobody used it anyway. Bye, Tassilo ___ auctex mailing list auctex@gnu.org https://lists.gnu.org/mailman/listinfo/auctex
Re: [AUCTeX] Patch to tex-info.el to support node name completion in @..ref commands, and solve a few other pbs
> Hi guys, > Hello Tassilo, > I'm somehow lost. It seems that some mail, presumably Stefan -> > Vincent, started this discussion but that hasn't been delivered to me. > So well, yes, `pushnew' does the right thing, and in Vincent's patch > to tex-info.el, I replaced his `add-to-list' call with (pushnew > ... :test #'equal) anyhow. Another discussion has started concerning the docstring of cl-pushnew. Just to come back to the initial point concerning revision of function Texinfo-make-node-list, what has happened is that I was not aware of the rationale behind replacing add-to-list by pushnew, Stefan has clarified that add-to-list is not good for variables bound by a let statement and that this was the reason. I was speculating that there was an execution speed issue, and on top of that I had misunderstood that pushnew checks duplication against the whole list. That is why I had proposed a revision. > > If there's something else I have to do, please tell me. Indeed, this revision which I had proposed seems to be useless as far as execution speed is concerned --- at least that was not why Stefan did the change, and nobody had complained. However it still have some added value in terms of warning the user in case that a duplicate node name is detected. So, I am not really opinionated whether this further change is needed, it is up to you... Please note that I have just made some evaluation of the speed difference for 1225 nodes in a file that contrains only @node statements, on my machine the current implementation takes 0.04s, while the proposed revision 0.01s, although it is 4 times as fast, I cannot say that speed on its own is an issue worth the change. All the more that the difference would certainly be smaller in a buffer where nodes are not empty. VBR, Vincent Belaïche ___ auctex mailing list auctex@gnu.org https://lists.gnu.org/mailman/listinfo/auctex
Re: [AUCTeX] Patch to tex-info.el to support node name completion in @..ref commands, and solve a few other pbs
Joost Kremers writes: > On Di, Okt 27 2015, Tassilo Horn wrote: >> A bit too long given that the first sentence of a docstring should fit >> into one line and still mention all parameters. > > Just a nit pick, but AFAIK the convention is that the first line > should summarise what the function does, but there is no requirement > that it also mentions all variables that a function takes. Right, but in general that goes hand in hand with the "main" parameters, e.g., with `pushnew' X and PLACE are surely relevant for such a summary whereas the additional keyword arguments it takes are not. Bye, Tassilo ___ auctex mailing list auctex@gnu.org https://lists.gnu.org/mailman/listinfo/auctex
Re: [AUCTeX] Patch to tex-info.el to support node name completion in @..ref commands, and solve a few other pbs
Joost Kremers writes: > On Di, Okt 27 2015, Tassilo Horn wrote: >> A bit too long given that the first sentence of a docstring should fit >> into one line and still mention all parameters. > > Just a nit pick, but AFAIK the convention is that the first line should > summarise what the function does, but there is no requirement that it > also mentions all variables that a function takes. (Well, that's to say, > I don't always do this in my Elisp files and flycheck never complains > about it... ;-) The entire DOC string should mention all parameters in order. The "in order" criterion does not make a whole lot of sense to me really since the order of parameters is often determined by technical considerations rather than how one would like to arrange a DOC string grammatically, but there you are. -- David Kastrup ___ auctex mailing list auctex@gnu.org https://lists.gnu.org/mailman/listinfo/auctex
Re: [AUCTeX] Patch to tex-info.el to support node name completion in @..ref commands, and solve a few other pbs
On Di, Okt 27 2015, Tassilo Horn wrote: > A bit too long given that the first sentence of a docstring should fit > into one line and still mention all parameters. Just a nit pick, but AFAIK the convention is that the first line should summarise what the function does, but there is no requirement that it also mentions all variables that a function takes. (Well, that's to say, I don't always do this in my Elisp files and flycheck never complains about it... ;-) -- Joost Kremers Life has its moments ___ auctex mailing list auctex@gnu.org https://lists.gnu.org/mailman/listinfo/auctex
Re: [AUCTeX] Patch to tex-info.el to support node name completion in @..ref commands, and solve a few other pbs
Stefan Monnier writes: Hi guys, I'm somehow lost. It seems that some mail, presumably Stefan -> Vincent, started this discussion but that hasn't been delivered to me. So well, yes, `pushnew' does the right thing, and in Vincent's patch to tex-info.el, I replaced his `add-to-list' call with (pushnew ... :test #'equal) anyhow. If there's something else I have to do, please tell me. >> Ok, I should have read the *Help* more carefully, it states >> (cl-pushnew X PLACE): insert X at the head of the list if not already >> there. > >> and I understood "there" as meaning "head of the list", instead of >> just "list". Indeed, that's not overly clear. >> Of course the second sentence clarifies the point, but I overlooked >> it. > >> Maybe it could be rephrased as: > >> (cl-pushnew X PLACE): insert X at the head of the list PLACE unless X is >> already an element of PLACE. > >> If you agree with the rephrasal, I can implement the change. > > Fine by me, A bit too long given that the first sentence of a docstring should fit into one line and still mention all parameters. But why does the docstring start with a synopsis? I'd just delete that and say Insert X as head of list PLACE unless PLACE already contains X. Bye, Tassilo ___ auctex mailing list auctex@gnu.org https://lists.gnu.org/mailman/listinfo/auctex
Re: [AUCTeX] Patch to tex-info.el to support node name completion in @..ref commands, and solve a few other pbs
> Ok, I should have read the *Help* more carefully, it states > (cl-pushnew X PLACE): insert X at the head of the list if not already > there. > and I understood "there" as meaning "head of the list", instead of just > "list". Of course the second sentence clarifies the point, but > I overlooked it. > Maybe it could be rephrased as: > (cl-pushnew X PLACE): insert X at the head of the list PLACE unless X is > already an element of PLACE. > If you agree with the rephrasal, I can implement the change. Fine by me, Stefan "not the maintainer any more" ___ auctex mailing list auctex@gnu.org https://lists.gnu.org/mailman/listinfo/auctex
Re: [AUCTeX] Patch to tex-info.el to support node name completion in @..ref commands, and solve a few other pbs
Ooops... Sorry for the multiple sending of exactly the same email... Vincent. > From: vincent@hotmail.fr > To: monn...@iro.umontreal.ca > Date: Mon, 26 Oct 2015 23:46:45 +0100 > CC: auctex@gnu.org; t...@gnu.org > Subject: Re: [AUCTeX] Patch to tex-info.el to support node name completion in > @..ref commands, and solve a few other pbs > > Ok, I should have read the *Help* more carefully, it states > > >(cl-pushnew X PLACE): insert X at the head of the list if not already > there. > > and I understood "there" as meaning "head of the list", instead of just > "list". Of course the second sentence clarifies the point, but I overlooked > it. > > Maybe it could be rephrased as: > >(cl-pushnew X PLACE): insert X at the head of the list PLACE unless X is > already an element of PLACE. > > If you agree with the rephrasal, I can implement the change. > > Vincent. > > >> From: monn...@iro.umontreal.ca >> To: vincent@hotmail.fr >> CC: t...@gnu.org; auctex@gnu.org >> Subject: Re: [AUCTeX] Patch to tex-info.el to support node name completion >> in @..ref commands, and solve a few other pbs >> Date: Mon, 26 Oct 2015 13:34:57 -0400 >> >>> interest. Some time ago Stefan Monnier changed add-to-list by pushnew, I >>> don't know what the motivation was, probably somebody had complained >>> that add-to-list was taking too much time. >> >> add-to-list doesn't work on lexical variables. >> >>> However these are not functionally equivalent, pushnew will skip the >>> replacement only if the node name is already there on top of the list, >>> while add-to-list will skip it if it is already there at any place. >> >> Not so: (cl-)pushnew looks at the whole list as well. >> >> I don't have much opinion on this function in general, since I don't >> use it. All I care about is to not re-introduce the >> add-to-list abomination (it's fine for use in a user's ~/.emacs to add >> some element to a global variable, but definitely not when operating on >> a let-bound variable). >> >> >> Stefan > > ___ > auctex mailing list > auctex@gnu.org > https://lists.gnu.org/mailman/listinfo/auctex ___ auctex mailing list auctex@gnu.org https://lists.gnu.org/mailman/listinfo/auctex
Re: [AUCTeX] Patch to tex-info.el to support node name completion in @..ref commands, and solve a few other pbs
Ok, I should have read the *Help* more carefully, it states (cl-pushnew X PLACE): insert X at the head of the list if not already there. and I understood "there" as meaning "head of the list", instead of just "list". Of course the second sentence clarifies the point, but I overlooked it. Maybe it could be rephrased as: (cl-pushnew X PLACE): insert X at the head of the list PLACE unless X is already an element of PLACE. If you agree with the rephrasal, I can implement the change. Vincent. > From: monn...@iro.umontreal.ca > To: vincent@hotmail.fr > CC: t...@gnu.org; auctex@gnu.org > Subject: Re: [AUCTeX] Patch to tex-info.el to support node name completion in > @..ref commands, and solve a few other pbs > Date: Mon, 26 Oct 2015 13:34:57 -0400 > >> interest. Some time ago Stefan Monnier changed add-to-list by pushnew, I >> don't know what the motivation was, probably somebody had complained >> that add-to-list was taking too much time. > > add-to-list doesn't work on lexical variables. > >> However these are not functionally equivalent, pushnew will skip the >> replacement only if the node name is already there on top of the list, >> while add-to-list will skip it if it is already there at any place. > > Not so: (cl-)pushnew looks at the whole list as well. > > I don't have much opinion on this function in general, since I don't > use it. All I care about is to not re-introduce the > add-to-list abomination (it's fine for use in a user's ~/.emacs to add > some element to a global variable, but definitely not when operating on > a let-bound variable). > > > Stefan ___ auctex mailing list auctex@gnu.org https://lists.gnu.org/mailman/listinfo/auctex
Re: [AUCTeX] Patch to tex-info.el to support node name completion in @..ref commands, and solve a few other pbs
Ok, I should have read the *Help* more carefully, it states (cl-pushnew X PLACE): insert X at the head of the list if not already there. and I understood "there" as meaning "head of the list", instead of just "list". Of course the next sentence in the docstring clarifies the point, but I overlooked it. Maybe it could be rephrased as: (cl-pushnew X PLACE): insert X at the head of list PLACE unless X is already an element of PLACE. If you agree with the rephrasal, I can implement the change. Vincent. > From: monn...@iro.umontreal.ca > To: vincent@hotmail.fr > CC: t...@gnu.org; auctex@gnu.org > Subject: Re: [AUCTeX] Patch to tex-info.el to support node name completion in > @..ref commands, and solve a few other pbs > Date: Mon, 26 Oct 2015 13:34:57 -0400 > >> interest. Some time ago Stefan Monnier changed add-to-list by pushnew, I >> don't know what the motivation was, probably somebody had complained >> that add-to-list was taking too much time. > > add-to-list doesn't work on lexical variables. > >> However these are not functionally equivalent, pushnew will skip the >> replacement only if the node name is already there on top of the list, >> while add-to-list will skip it if it is already there at any place. > > Not so: (cl-)pushnew looks at the whole list as well. > > I don't have much opinion on this function in general, since I don't > use it. All I care about is to not re-introduce the > add-to-list abomination (it's fine for use in a user's ~/.emacs to add > some element to a global variable, but definitely not when operating on > a let-bound variable). > > > Stefan ___ auctex mailing list auctex@gnu.org https://lists.gnu.org/mailman/listinfo/auctex
Re: [AUCTeX] Patch to tex-info.el to support node name completion in @..ref commands, and solve a few other pbs
> interest. Some time ago Stefan Monnier changed add-to-list by pushnew, I > don't know what the motivation was, probably somebody had complained > that add-to-list was taking too much time. add-to-list doesn't work on lexical variables. > However these are not functionally equivalent, pushnew will skip the > replacement only if the node name is already there on top of the list, > while add-to-list will skip it if it is already there at any place. Not so: (cl-)pushnew looks at the whole list as well. I don't have much opinion on this function in general, since I don't use it. All I care about is to not re-introduce the add-to-list abomination (it's fine for use in a user's ~/.emacs to add some element to a global variable, but definitely not when operating on a let-bound variable). Stefan ___ auctex mailing list auctex@gnu.org https://lists.gnu.org/mailman/listinfo/auctex
Re: [AUCTeX] Patch to tex-info.el to support node name completion in @..ref commands, and solve a few other pbs
Hello Tassilo, To Stefan also concerning add-to-list -> pushnew change... Feedback inserted below... > From: t...@gnu.org > To: vincent@hotmail.fr > Subject: Re: [AUCTeX] Patch to tex-info.el to support node name completion in > @..ref commands, and solve a few other pbs > Date: Mon, 26 Oct 2015 14:16:20 +0100 > > Vincent Belaïche writes: > > Hi Vincent, > >> Ooops... sorry, I hadn't noticed that you also had switched to git. > > Hey, that has been in April 2013! :-) Ok, I understand what has happened. I am using some script to do all the refreshing of the work area, and installation, when I had modified this script from SVN to GIT quite some time ago I was not so familiar with git at that time, and I wrote `git fetch' instead of `git pull' for the case of already existing work area... > >> I will do the checks ASAP and let you know. > > Great, thank you. I think the only hunk that didn't apply is the one > adapting `Texinfo-insert-node' so have a special look on that. > I had a look at function Texinfo-make-node-list, which is the one of interest. Some time ago Stefan Monnier changed add-to-list by pushnew, I don't know what the motivation was, probably somebody had complained that add-to-list was taking too much time. However these are not functionally equivalent, pushnew will skip the replacement only if the node name is already there on top of the list, while add-to-list will skip it if it is already there at any place. So, I assume that the motivation was to avoid any duplicate in the resulting list. Anyways node names are supposed to be unique, so whether it be pushnew or add-to-list that should make a difference only in the case of boguous Texinfo source code Another problem is that the user is not warned if there are duplicate node names, why doing this check if then things go by silently. So I propose to revise the code as follows: (defun Texinfo-make-node-list (&optional nodes) ;; Build list of nodes in current buffer. ;; (What about using `imenu--index-alist'?) ;; FIXME: Support multi-file documents. (let ((htable (make-hash-table :test 'equal)) hentry pt key msg) (dolist (n nodes) (puthash (car n) t htable)) (save-excursion (goto-char (point-min)) (while (re-search-forward "^@node\\b" nil t) (skip-chars-forward "[:blank:]") (setq key (Texinfo-nodename-de-escape (buffer-substring-no-properties (setq pt (point)) (progn (skip-chars-forward "^\r\n,") (skip-chars-backward "[:blank:]") (point hentry (gethash key htable)) (cond ((integerp hentry) (push (format "AUCTeX/Texinfo:%d: Duplicate node name `%s' at line %d" (line-number-at-pos hentry) key (line-number-at-pos pt)) msg)) ((eq hentry t) (puthash key pt htable)) (t (puthash key pt htable) (push (list key) nodes) (if msg (warn "AUCTeX/Texinfo...\n%s" (mapconcat 'identity (nreverse msg) "\n" nodes) The changes are as follows: - use hash table, to keep both speed advantage of pushnew and thorough duplicate check of add-to-list - warn the user if a duplicate is found > Bye, > Tassilo Bye, Vincent ___ auctex mailing list auctex@gnu.org https://lists.gnu.org/mailman/listinfo/auctex
Re: [AUCTeX] Patch to tex-info.el to support node name completion in @..ref commands, and solve a few other pbs
Vincent Belaïche writes: Hi Vincent, > I would like to submit to you the attached patch. Very cool, thanks a lot. > + [snip] > + > 2014-09-13 Vincent Belaïche As you see, the patch was against a pretty old version and didn't apply anymore cleanly. I think I was able to merge it correctly but in the future it would be great if you could base your patches on the Git version. Please validate that I didn't screw up things in http://git.savannah.gnu.org/cgit/auctex.git/commit/?id=d56502e7dcba118c62d38f6fe2f5a855834e6816 Thanks again! Tassilo ___ auctex mailing list auctex@gnu.org https://lists.gnu.org/mailman/listinfo/auctex
