unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
From: Philip Kaludercic <philipk@posteo.net>
To: "Matúš Goljer" <matus.goljer@gmail.com>
Cc: emacs-devel@gnu.org
Subject: Re: [PATCH] seq.el: add seq-last for symmetry with seq-first
Date: Tue, 14 Mar 2023 16:14:18 +0000	[thread overview]
Message-ID: <875yb3cntx.fsf@posteo.net> (raw)
In-Reply-To: <87wn3jl4f8.fsf@gmail.com> ("Matúš Goljer"'s message of "Tue, 14 Mar 2023 16:48:43 +0100")

Matúš Goljer <matus.goljer@gmail.com> writes:

>> Wouldn't it make sense to add a specialised implementation for lists, to
>> avoid recusing the list twice.
>
> I can turn it into defgeneric with default implementation same as what I
> provided and special instance for a list, that makes sense.  Although I
> wonder what the performance impact is of the dispatch vs iterating a
> list /shrug.

That would be worth investigating, but considering that your proposed
implementation would already have two dispatches (seq-elt and
seq-length), I don't think this will be much worse, especially when the
sequence becomes longer.

>> A thing I notice is that seq-first is not consistent on the way it
>> behaves if the sequence is empty.
>>
>>         (seq-first '()) ;=> nil
>>
>> while
>>
>>         (seq-first [])
>>
>> raises an error.  seq-last would have the same issue for vectors, except
>> that it would attempt to index the position -1, which might be
>> confusing?
>
> I think for lists it should behave as `nth` or `elt`, so it gives nil.

But why?  Wouldn't that be a leaky abstraction, since the behaviour
doesn't consistently abstract over the concrete sequence types?  If code
doesn't want to worry about what sequence is being used, then it has to
manually check the return value or if a signal was raised, depending on
the type of the argument to seq-last (which is to ignore the issue that
we cannot distinguish between (seq-first '()) and (seq-first '(nil)),
the same also being the case for seq-last).

> I agree that the error with -1 for vector might be confusing.  Should we
> instead raise our own (seq) error for empty vector with seq-first and
> seq-last?  Not sure what would be the best way.

I believe this would always be better, even for seq-first, though it
might be that too late for that?

> I should also add some tests, I've noticed there was a test file for
> seq.  Should I resubmit this patch with the feedback incorporated to
> bug-gnu-emacs@gnu.org instead?  I'm not sure if emacs-devel is the
> place, sorry T_T.

IIRC patches should be sent to bug-gnu-emacs@gnu.org, but emacs-devel is
also acceptable if there is a need for discussion.  I think the points I
raised above would be worth a general discussion on how seq is to be
used, which is not the matter of a single bug report.



  reply	other threads:[~2023-03-14 16:14 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-14 12:55 [PATCH] seq.el: add seq-last for symmetry with seq-first Matúš Goljer
2023-03-14 15:26 ` Philip Kaludercic
2023-03-14 15:48   ` Matúš Goljer
2023-03-14 16:14     ` Philip Kaludercic [this message]
2023-03-14 18:35       ` Augusto Stoffel
2023-03-14 19:14         ` Matúš Goljer
2023-03-14 22:04         ` Philip Kaludercic
2023-03-15  7:06           ` Augusto Stoffel
2023-03-15  8:09             ` Philip Kaludercic

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

  List information: https://www.gnu.org/software/emacs/

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

  git send-email \
    --in-reply-to=875yb3cntx.fsf@posteo.net \
    --to=philipk@posteo.net \
    --cc=emacs-devel@gnu.org \
    --cc=matus.goljer@gmail.com \
    /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 public inbox

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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).