all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: "Milan Stanojević" <mstanojevic@janestreet.com>
To: Eli Zaretskii <eliz@gnu.org>
Cc: 31837@debbugs.gnu.org
Subject: bug#31837: 26.1; replace-buffer-contents doesn't work if buffer has multibyte characters
Date: Mon, 18 Jun 2018 16:29:41 -0400	[thread overview]
Message-ID: <CAArdBwP6n=QgMCRtN-R=XgnMY8-sHUJhYXRi2L6SuREYOQAUsw@mail.gmail.com> (raw)
In-Reply-To: <83sh5krmd1.fsf@gnu.org>

>> But what matters is whether there are actual multibyte characters in
>> the buffer
>
> If a buffer is unibyte, there could be no multibyte characters in it,
> by definition.
>
>> buf_charpos_to_bytepos already shortcircuits if there are none.
>
> Not before it calls some macros.  And it is confusing to call a
> conversion function when no conversion is needed, because the
> correctness of that can only be established by looking at the called
> conversion function.  That gets in the way of code reading, IME.

Right, I get your point regarding readability but maybe I can clarify
what I'm thinking.

The question is whether we need to optimize buffer_chars_equal  or not.

If not, then we can just call buf_charpos_to_bytepos without any other
checks, that seems most readable.

If we do care about optimizing it, then it is better to check whether
the buffer actually contains multibyte characters (I guess with
BUF_Z(buf) == BUF_Z_BYTE(buf)) than using enable_multibyte_characters
since the the former would catch strictly more cases when we don't
need charpos to bytepos conversion.
Also, it occurs to me, the check shouldn't be done in
buffer_chars_equal, since the same check would be done multiple times
for a single character (for every comparison of that character), it
should be done once and the result stored in ctx before calling
compareseq.
Again, I'm not sure this matters at all in practice, all the checks
are just a few memory accesses (presumable in the cache) and integer
comparisons.

Anyway, I could try doing something in the future about this, but the
code as it is now is perfectly fine.

Thanks again!





  reply	other threads:[~2018-06-18 20:29 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-14 21:34 bug#31837: 26.1; replace-buffer-contents doesn't work if buffer has multibyte characters Milan Stanojević
2018-06-15  8:34 ` Eli Zaretskii
2018-06-18 17:44   ` Milan Stanojević
2018-06-18 18:16     ` Eli Zaretskii
2018-06-18 20:29       ` Milan Stanojević [this message]
2018-06-19  2:30         ` Eli Zaretskii
2018-07-01 17:32   ` Philipp Stephani

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='CAArdBwP6n=QgMCRtN-R=XgnMY8-sHUJhYXRi2L6SuREYOQAUsw@mail.gmail.com' \
    --to=mstanojevic@janestreet.com \
    --cc=31837@debbugs.gnu.org \
    --cc=eliz@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.