unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
From: Eli Zaretskii <eliz@gnu.org>
To: Dmitry Gutov <dgutov@yandex.ru>
Cc: emacs-devel@gnu.org, mardani29@yahoo.es
Subject: Re: [PATCH] Fix some failing tests in BSD systems
Date: Mon, 04 Jan 2021 17:52:18 +0200	[thread overview]
Message-ID: <83pn2kbsy5.fsf@gnu.org> (raw)
In-Reply-To: <5d574d90-34b1-0fb1-8b3a-83e7709eadfc@yandex.ru> (message from Dmitry Gutov on Mon, 4 Jan 2021 15:10:40 +0200)

> Cc: mardani29@yahoo.es, emacs-devel@gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Mon, 4 Jan 2021 15:10:40 +0200
> 
> >> On 03.01.2021 19:35, Eli Zaretskii wrote:
> >>> Why are we producing file names by concatenation, instead of calling
> >>> expand-file-name?  Wouldn't using the latter solve the problem?  Or
> >>> what did I miss?
> >>
> >> While we sometimes do that (expand-file-name is not free, and for
> >> certain uses its overhead counts), that doesn't seem to be the case here.
> > 
> > Not sure what you mean: I clearly saw calls to concat in the patch,
> > both in the old and the new code.  What did I miss?
> 
> The changes you are referring to are both in the test code. There's 
> nothing inherently wrong in using concat in such code, as long as we 
> test what we want to test.

There's nothing wrong with that, sure, but you risk false failures due
to issues like the one here.  However, if you are okay with that, I
don't mind.  It just was strange to see such code.

> If you actually meant to suggest an alternative comparison strategy 
> (like in Daniel's latest patch), then I think the test becomes 
> unnecessarily lax. After all, we want to also check how the string 
> "looks" (because the user will see it verbatim), not just what file it 
> points at.

I don't think you can reliably know how the file name will be
formatted by a particular variety of 'find', not IME.  But I'm not
going to argue about this.



  reply	other threads:[~2021-01-04 15:52 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <m1sg7i7xfl.fsf.ref@yahoo.es>
2021-01-03 17:16 ` [PATCH] Fix some failing tests in BSD systems Daniel Martín
2021-01-03 17:35   ` Eli Zaretskii
2021-01-03 21:08     ` Daniel Martín
2021-01-04  1:41     ` Dmitry Gutov
2021-01-04  3:30       ` Eli Zaretskii
2021-01-04 13:10         ` Dmitry Gutov
2021-01-04 15:52           ` Eli Zaretskii [this message]
2021-01-04 17:10             ` Dmitry Gutov
2021-01-04 22:25               ` Daniel Martín
2021-01-06  1:38                 ` Dmitry Gutov
2021-01-06  9:41                   ` Daniel Martín
2021-01-06 17:17                     ` Dmitry Gutov
2021-01-06 18:48                       ` Stefan Monnier
2021-01-06 21:17                         ` Dmitry Gutov
2021-01-04  1:43   ` Dmitry Gutov
2021-01-04  3:48     ` Stefan Monnier

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=83pn2kbsy5.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=dgutov@yandex.ru \
    --cc=emacs-devel@gnu.org \
    --cc=mardani29@yahoo.es \
    /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).