GNU bug report logs - #5012
avl-tree.el enhancements

Previous Next

Full log

View this message in rfc822 format

From: help-debbugs <at> gnu.org (GNU bug Tracking System)
To: Toby Cubitt <toby-predictive-dated-1259427171.f81182 <at> dr-qubit.org>
Subject: bug#5024: closed (Re: bug#5012: avl-tree.el enhancements)
Date: Thu, 23 Feb 2012 19:55:03 +0000

[Message part 1 (text/plain, inline)]

Your bug report

#5012: avl-tree.el enhancements

which was filed against the emacs package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to 5024 <at> debbugs.gnu.org.

-- 
5012: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=5012
GNU Bug Tracking System
Contact help-debbugs <at> gnu.org with problems

[Message part 2 (message/rfc822, inline)]

From: Glenn Morris <rgm <at> gnu.org>
To: 5012-done <at> debbugs.gnu.org
Subject: Re: bug#5012: avl-tree.el enhancements
Date: Thu, 23 Feb 2012 14:51:33 -0500

Version: 24.1

Installed 2011-05-27.

[Message part 3 (message/rfc822, inline)]

From: Toby Cubitt <tsc25 <at> cantab.net>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: bug-gnu-emacs <at> gnu.org
Subject: Re: avl-tree.el enhancements
Date: Mon, 23 Nov 2009 16:59:08 +0000

[Message part 4 (text/plain, inline)]

On Mon, Nov 23, 2009 at 09:58:08AM -0500, Stefan Monnier wrote:
> >> Hmm... using integers for "direction" isn't pretty.  I see it mostly
> >> comes from its use in avl-tree--node-branch.  I guess we'll have to live
> >> with it for now...
>
> > Exactly -- it stems from avl-tree--node-branch (which isn't my code).
> > The comment after its definition (also not mine) explains the reason
> > for it.  I can't think of an efficient way to avoid this.
>
> I know.  I wrote the comment ;-)

:-)

> > I personally find the fact that internal library functions aren't
> > documented to be a useful form of documentation in itself: it tells me
> > that the function definitely shouldn't be used outside of the library.
>
> That's what the "--" is for in avl-tree--del-balance.  It's also much
> better because you don't need to check "does this function have
> a docstring" to figure out whether it's internal or not.

I see. I wasn't aware that the "--" convention was widely followed for
internal library functions (at least, widely enough to be understood). In
that case, you're of course right.

Is this convention documented anywhere? Should it be? I can't find it
mentioned anywhere in Appendix D of Emacs Lisp Manual...

> > If you still insist on making one or other or both of these into
> > docstrings, I'll make the necessary changes to the patches.
>
> Please do, thank you.

I've fixed this in the new versions of the patches (attached). Since my
changes touch a lot of functions in avl-tree.el, I've gone through and
changed as many comments to docstrings as I can (I think I've caught all
of them, in fact).

> >> Why exactly do you remove the \(fn TREE) thingy at the end?
>
> > It seemed to be a strange documentation convention followed only by
> > avl-tree.el, and inconsistently at that. It's redundant, and as far as I
> > know isn't used anywhere else in Emacs, because "C-h f <function>"
> > already automatically shows the function's calling convention.
>
> It's a trick that can be used to *change* the calling
> convention displayed.  So it's not used/needed often.  But here, for
> example, it's used to change (avl-tree-compare-function CL-X) into
> (avl-tree-compare-function TREE), so the docstring (which refers to
> TREE) makes more sense.
> Sometimes it's used to hide some internal arguments.
> It's also used systematically in autoloaded definitions so as to provide
> the calling convention before the function is loaded.

Always good to learn something new about Emacs :) I've reverted this
change in the attached patches. Now I'll have to go back through my other
Elisp code to see if this trick would be useful!

> >> Overall, looks good.  Please provide a ChangeLog entry for it as well.
>
> > Errrmmmm...do you mean a ChangeLog entry in the avl-tree.el file itself?
> > If so, I already did! :)
>
> No, an entry in the format used by the ChangeLog file.
> See http://www.gnu.org/prep/standards/standards.html#Change-Logs

Ah, OK. I've attached a ChangeLog file with the entries for these
changes. I've tried to follow the guidelines in the link, but you'd
better check and let me know if they're OK.

Toby

[avl-tree.el_ChangeLog (text/plain, attachment)]

[avl-tree.el.1.diff (text/plain, attachment)]

[avl-tree.el.2.diff (text/plain, attachment)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.