all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Eli Zaretskii <eliz@gnu.org>
To: Chong Yidong <cyd@gnu.org>
Cc: 12314@debbugs.gnu.org
Subject: bug#12314: 24.2.50; `add-to-history': use `setq' with `delete'
Date: Sun, 09 Sep 2012 20:25:56 +0300	[thread overview]
Message-ID: <831uibm7kr.fsf@gnu.org> (raw)
In-Reply-To: <87r4qbr5s1.fsf@gnu.org>

> From: Chong Yidong <cyd@gnu.org>
> Cc: Drew Adams <drew.adams@oracle.com>,  12314@debbugs.gnu.org
> Date: Sun, 09 Sep 2012 15:53:34 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > But the manual should cater first and foremost to newbies.  The rest
> > will get the point when they read the detailed description of how the
> > list is modified.
> 
> I modified the manual to hopefully make the situation clearer.  In
> particular, the descriptions of delq and delete explicitly say that you
> typically ought to use the return value.

Thanks, the text is much better now.  I still think the "destructive
modification" part should be retired to later in the description,
perhaps as notes, because it invokes mental models that get in the way
of interpreting the text correctly.  But I won't fight over it.

> The docstrings are harder, since they should be succinct.  Here is what
> I suggest; WDYT?
> 
> 
> (delq ELT LIST)
> 
> Delete by side effect occurrences of ELT as a member of LIST.
> Comparison is done with `eq'.  Return the resulting list.
> 
> More precisely, this function skips any occurrences of ELT at the
> front of LIST, then removes occurrences of ELT from the remaining
> sublist by modifying the list structure, then returns the resulting
> sublist.
> 
> Therefore, write `(setq foo (delq element foo))' to be sure of
> changing the value of `foo'.

I would remove the "by side effect" part, as it doesn't really add
anything of importance, and OTOH runs a real risk of confusing the
reader.  Otherwise, I think this is fine.  Thanks.

> (delete ELT SEQ)
> 
> Delete occurrence of ELT as a member of SEQ.
> SEQ must be a sequence (i.e. a list, a vector, or a string).
> Comparison is done with `equal'.  Return the resulting sequence.
> 
> If SEQ is a list, this behaves like `delq', except that it compares
> with `equal' instead of `eq'.  In particular, it may remove elements
> by altering the list structure.
> 
> If SEQ is not a list, deletion is not a side effect; this function
> creates and returns a new sequence.
> 
> Therefore, write `(setq foo (delete element foo))'
> to be sure of changing the value of `foo'.

This is also OK, except that I'd prefer an explicit description to a
reference to 'delq' in the second paragraph.  The corresponding text
in the doc string of 'delq' is short enough, so there are no real
savings in the reference, while the disadvantage of having to consult
another doc string is real.





  reply	other threads:[~2012-09-09 17:25 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-30 23:08 bug#12314: 24.2.50; `add-to-history': use `setq' with `delete' Drew Adams
2012-09-08 14:32 ` Chong Yidong
2012-09-08 14:43   ` Eli Zaretskii
2012-09-08 14:57     ` Drew Adams
2012-09-08 15:20       ` Eli Zaretskii
2012-09-08 15:48         ` Drew Adams
2012-09-08 16:05           ` Eli Zaretskii
2012-09-08 16:19             ` Andreas Schwab
2012-09-08 16:33               ` Eli Zaretskii
2012-09-08 16:50                 ` Lars Ingebrigtsen
2012-09-08 16:54                 ` Drew Adams
2012-09-08 17:06                 ` Andreas Schwab
2012-09-08 16:35               ` Drew Adams
2012-09-08 16:25             ` Drew Adams
2012-09-08 16:32               ` Eli Zaretskii
2012-09-08 16:42                 ` Drew Adams
2012-09-08 21:21                   ` Eli Zaretskii
2012-09-08 22:26                     ` Drew Adams
2012-09-09  3:00                       ` Eli Zaretskii
2012-09-09  6:29                         ` Drew Adams
2012-09-09  7:53                         ` Chong Yidong
2012-09-09 17:25                           ` Eli Zaretskii [this message]
2012-09-10 11:54                       ` Wolfgang Jenkner
2012-09-08 23:11                     ` Stefan Monnier
2012-09-09  2:51                       ` Eli Zaretskii
2012-09-09 14:44                         ` Stefan Monnier
2012-09-09 17:14                           ` Eli Zaretskii
2012-09-09 17:35                             ` Drew Adams
2012-09-09 18:20                               ` Eli Zaretskii
2012-09-09 19:46                                 ` Drew Adams
2012-09-09 21:37                             ` Stefan Monnier
2012-09-10  4:37                               ` Eli Zaretskii
2012-09-10 12:59                                 ` Stefan Monnier
2012-09-10 15:01                                   ` Stefan Monnier
2012-09-10 15:21                                   ` Drew Adams
2012-09-10 16:24                                   ` Eli Zaretskii
2012-09-09  8:25 ` Dmitry Gutov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=831uibm7kr.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=12314@debbugs.gnu.org \
    --cc=cyd@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.