unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Eli Zaretskii <eliz@gnu.org>
To: "Mattias Engdegård" <mattias.engdegard@gmail.com>
Cc: 58168@debbugs.gnu.org, larsi@gnus.org
Subject: bug#58168: string-lessp glitches and inconsistencies
Date: Fri, 07 Oct 2022 18:33:57 +0300	[thread overview]
Message-ID: <83leprmyvu.fsf@gnu.org> (raw)
In-Reply-To: <58016C43-E429-4EF1-9660-0F29CA36B6FE@gmail.com> (message from Mattias Engdegård on Fri, 7 Oct 2022 16:45:57 +0200)

> From: Mattias Engdegård <mattias.engdegard@gmail.com>
> Date: Fri, 7 Oct 2022 16:45:57 +0200
> Cc: larsi@gnus.org,
>  58168@debbugs.gnu.org
> 
> > I don't think it matters much, because whatever we produce we cannot
> > be sure it will look identical to the original format string.
> 
> I agree. What about this patch then?
> 
> diff --git a/src/editfns.c b/src/editfns.c
> index c1414071c7..5a99a40656 100644
> --- a/src/editfns.c
> +++ b/src/editfns.c
> @@ -3551,10 +3551,15 @@ styled_format (ptrdiff_t nargs, Lisp_Object *args, bool message)
>  		      || float_conversion || conversion == 'i'
>  		      || conversion == 'o' || conversion == 'x'
>  		      || conversion == 'X'))
> -	    error ("Invalid format operation %%%c",
> -		   multibyte_format
> -		   ? STRING_CHAR ((unsigned char *) format - 1)
> -		   : *((unsigned char *) format - 1));
> +	    {
> +	      unsigned char *p = (unsigned char *) format - 1;
> +	      if (multibyte_format)
> +		error ("Invalid format operation %%%c", STRING_CHAR (p));
> +	      else
> +		error ((*p <= 127 ? "Invalid format operation %%%c"
> +			: "Invalid format operation char #x%02x"),
> +		       *p);
> +	    }
>  	  else if (! (FIXNUMP (arg) || ((BIGNUMP (arg) || FLOATP (arg))
>  					&& conversion != 'c')))
>  	    error ("Format specifier doesn't match argument type");

I'd prefer to show it in octal, not in hex, since that's the default
in Emacs.

> > A test suite doesn't have to
> > assume that the internals work as they should, it should just test
> > that.  So testing both sounds to me better than testing just one
> > assuming that this one covers both.
> 
> I agree, we should definitely test raw bytes inserted from both unibyte and multibyte strings. The current code doesn't do that, hence my patch.
> 
> With respect to other values there should be a reasonable qualitative difference between test cases: testing both #x80 and #xfc isn't more useful than testing the display of both the letters 'A' and 'B'. But if you think that the current code is deficient in coverage, please do propose extensions and I'll adapt the patch accordingly.

I think the more values we test the better.  The codepoints between
#x80 and #xA0 in particular are treated differently than those above
#xA0, so at least one from each range should be beneficial.





  reply	other threads:[~2022-10-07 15:33 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-29 16:24 bug#58168: string-lessp glitches and inconsistencies Mattias Engdegård
2022-09-29 17:00 ` Mattias Engdegård
2022-09-29 17:11 ` Eli Zaretskii
2022-09-30 20:04   ` Mattias Engdegård
2022-10-01  5:22     ` Eli Zaretskii
2022-10-01 19:57       ` Mattias Engdegård
2022-10-02  5:36         ` Eli Zaretskii
2022-10-03 19:48           ` Mattias Engdegård
2022-10-04  5:55             ` Eli Zaretskii
2022-10-04 17:40               ` Richard Stallman
2022-10-04 18:07                 ` Eli Zaretskii
2022-10-06  9:05               ` Mattias Engdegård
2022-10-06 11:06                 ` Eli Zaretskii
2022-10-07 14:23                   ` Mattias Engdegård
2022-10-08  7:35                     ` Eli Zaretskii
2022-10-14 14:39                       ` Mattias Engdegård
2022-10-14 15:31                         ` Eli Zaretskii
2022-10-17 12:44                           ` Mattias Engdegård
2022-09-30 13:52 ` Lars Ingebrigtsen
2022-09-30 20:12   ` Mattias Engdegård
2022-10-01  5:34     ` Eli Zaretskii
2022-10-01 11:51       ` Mattias Engdegård
2022-10-01 10:02     ` Lars Ingebrigtsen
2022-10-01 10:12       ` Eli Zaretskii
2022-10-01 13:37       ` Mattias Engdegård
2022-10-01 13:43         ` Lars Ingebrigtsen
2022-10-03 19:48           ` Mattias Engdegård
2022-10-04 10:44             ` Lars Ingebrigtsen
2022-10-04 11:37             ` Eli Zaretskii
2022-10-04 14:44               ` Mattias Engdegård
2022-10-04 16:24                 ` Eli Zaretskii
2022-10-06  9:05                   ` Mattias Engdegård
2022-10-06 11:13                     ` Eli Zaretskii
2022-10-06 12:43                       ` Mattias Engdegård
2022-10-06 14:34                         ` Eli Zaretskii
2022-10-07 14:45                           ` Mattias Engdegård
2022-10-07 15:33                             ` Eli Zaretskii [this message]
2022-10-08 17:13                               ` Mattias Engdegård
2022-10-01 13:51         ` Eli Zaretskii
2022-10-01  5:30   ` Eli Zaretskii

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=83leprmyvu.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=58168@debbugs.gnu.org \
    --cc=larsi@gnus.org \
    --cc=mattias.engdegard@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).