GNU bug report logs - #38851
27.0.50; Recent my patch breaks isearch with macOS native input method

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 38851 in the body.
You can then email your comments to 38851 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: tsuucat <tsuucat <at> icloud.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 27.0.50; Recent my patch breaks isearch with macOS native input method
Date: Thu, 2 Jan 2020 03:25:13 +0900

[Message part 1 (text/plain, inline)]

I sent a patch for Bug#23412 and the patch was already merged.
(http://git.savannah.gnu.org/cgit/emacs.git/commit/?h=emacs-27&id=ba042176d8931cdf9441b3b4919ec74b75019494)

Unfortunately, the patch breaks isearch with macOS native input method.
I tried to modify ns-echo-working-text function and ns-delete-working-text
function, but I cannot do it.
This is because input-pending-p now returns t after pressing RET to confirm 
the conversion. (isearch-update function uses input-pending-p)

The following patch treats ns-unput-working-text event by deleteWorkingText 
specially in read_char(). This solve the problem.

[0001-Change-redisplay-solution.patch (application/octet-stream, attachment)]

[Message part 3 (text/plain, inline)]

--
tsuucat

Message #8 received at 38851 <at> debbugs.gnu.org (full text, mbox):

From: Alan Third <alan <at> idiocy.org>
To: tsuucat <tsuucat <at> icloud.com>
Cc: 38851 <at> debbugs.gnu.org
Subject: Re: bug#38851: 27.0.50; Recent my patch breaks isearch with macOS
 native input method
Date: Fri, 3 Jan 2020 20:29:43 +0000

On Thu, Jan 02, 2020 at 03:25:13AM +0900, tsuucat via Bug reports for GNU Emacs, the Swiss army knife of text editors wrote:
> I sent a patch for Bug#23412 and the patch was already merged.
> (http://git.savannah.gnu.org/cgit/emacs.git/commit/?h=emacs-27&id=ba042176d8931cdf9441b3b4919ec74b75019494)
> 
> Unfortunately, the patch breaks isearch with macOS native input method.
> I tried to modify ns-echo-working-text function and ns-delete-working-text
> function, but I cannot do it.
> This is because input-pending-p now returns t after pressing RET to confirm 
> the conversion. (isearch-update function uses input-pending-p)
> 
> The following patch treats ns-unput-working-text event by deleteWorkingText 
> specially in read_char(). This solve the problem.

Thanks. I have no better ideas, so this will have to do. Does anyone
else have any opinions?
-- 
Alan Third

Message #11 received at 38851 <at> debbugs.gnu.org (full text, mbox):

From: tsuucat <tsuucat <at> icloud.com>
To: Alan Third <alan <at> idiocy.org>
Cc: 38851 <at> debbugs.gnu.org
Subject: Re: bug#38851: 27.0.50; Recent my patch breaks isearch with macOS
 native input method
Date: Tue, 11 Feb 2020 02:08:14 +0900

[Message part 1 (text/plain, inline)]

> Thanks. I have no better ideas, so this will have to do. Does anyone
> else have any opinions?

I have some more explanation. After this commit (https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=9e77c1b7bcfd0807be7fe67daf73c2320e864309 <https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=9e77c1b7bcfd0807be7fe67daf73c2320e864309>)
which introduce input_was_pending, read_char became to redisplay always 
after processing special event.

```
input_was_pending = input_pending;
```
is a way to prevent redisplay. The same code is placed at end of read_char.


Emacs at current emacs-27 branch cannot use isearch with macOS native 
input method (I checked with macOS Japanese input method and get 
```
ns-delete-working-text: Wrong type argument: arrayp, nil
```
).  This is worse situation for macOS native input method users.
Even if modifying keyboard.c is not acceptable, I think reverting the position of 
```
  if (workingText != nil)
    [self deleteWorkingText];
```
in insertText should be done.

If the patch I sent before is acceptable, I have two patch for improvement.

0001-proper-selection-window-position-for-isearch.patch
This is to show candidate window for IM at proper position when isearch.


0001-Remove-unfavorable-deleteWorkingText.patch
Currently clicking buffer or focusing out frame delete working text. To prevent
deleting working text by such a input.


--
 tsuucat

[Message part 2 (text/html, inline)]

[0001-proper-selection-window-position-for-isearch.patch (application/octet-stream, attachment)]

[Message part 4 (text/html, inline)]

[0001-Remove-unfavorable-deleteWorkingText.patch (application/octet-stream, attachment)]

[Message part 6 (text/html, inline)]

Message #14 received at 38851 <at> debbugs.gnu.org (full text, mbox):

From: Alan Third <alan <at> idiocy.org>
To: tsuucat <tsuucat <at> icloud.com>
Cc: 38851 <at> debbugs.gnu.org
Subject: Re: bug#38851: 27.0.50; Recent my patch breaks isearch with macOS
 native input method
Date: Mon, 10 Feb 2020 21:44:51 +0000

On Tue, Feb 11, 2020 at 02:08:14AM +0900, tsuucat wrote:
> 
> > Thanks. I have no better ideas, so this will have to do. Does anyone
> > else have any opinions?

Apologies, I forgot about this patch. I can see two problems here:

You probably need to sign the copyright assignment form before any
further patches can be applied.

It’s getting pretty far on in the Emacs 27 release cycle and I may
have left this too late.

IIRC your patch fixed a visual flicker rather than any actual
usability problem, is that right? Would we be better reverting it in
Emacs 27 and continuing with it and these further patches in the
master branch?

Eli, any opinion on this? The commit in question is

    ba042176d8931cdf9441b3b4919ec74b75019494
-- 
Alan Third

Message #17 received at 38851 <at> debbugs.gnu.org (full text, mbox):

From: tsuucat <tsuucat <at> icloud.com>
To: Alan Third <alan <at> idiocy.org>
Cc: 38851 <at> debbugs.gnu.org
Subject: Re: bug#38851: 27.0.50; Recent my patch breaks isearch with macOS
 native input method
Date: Tue, 11 Feb 2020 07:21:49 +0900

> You probably need to sign the copyright assignment form before any
> further patches can be applied.

I have already assigned copyright for GNU Emacs.

> IIRC your patch fixed a visual flicker rather than any actual
> usability problem, is that right?


Yes.

--
tsuucat

Message #20 received at 38851 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: tsuucat <tsuucat <at> icloud.com>
Cc: alan <at> idiocy.org, 38851 <at> debbugs.gnu.org
Subject: Re: bug#38851: 27.0.50;
 Recent my patch breaks isearch with macOS native input method
Date: Tue, 11 Feb 2020 17:56:45 +0200

> Date: Tue, 11 Feb 2020 07:21:49 +0900
> From: tsuucat via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> > You probably need to sign the copyright assignment form before any
> > further patches can be applied.
> 
> I have already assigned copyright for GNU Emacs.

Indeed, the assignment is already on file.

Thanks.

Message #23 received at 38851 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Alan Third <alan <at> idiocy.org>
Cc: 38851 <at> debbugs.gnu.org, tsuucat <at> icloud.com
Subject: Re: bug#38851: 27.0.50;
 Recent my patch breaks isearch with macOS native input method
Date: Tue, 11 Feb 2020 19:05:21 +0200

> Date: Mon, 10 Feb 2020 21:44:51 +0000
> From: Alan Third <alan <at> idiocy.org>
> Cc: 38851 <at> debbugs.gnu.org
> 
> Eli, any opinion on this? The commit in question is
> 
>     ba042176d8931cdf9441b3b4919ec74b75019494

I don't think I understand the change well enough, as it deals with NS
specific code.  But if it didn't fix the actual problem, perhaps it is
indeed better to work on it on master.

Message #26 received at 38851 <at> debbugs.gnu.org (full text, mbox):

From: Alan Third <alan <at> idiocy.org>
To: tsuucat <tsuucat <at> icloud.com>
Cc: 38851 <at> debbugs.gnu.org
Subject: Re: bug#38851: 27.0.50; Recent my patch breaks isearch with macOS
 native input method
Date: Tue, 11 Feb 2020 17:22:34 +0000

On Tue, Feb 11, 2020 at 07:21:49AM +0900, tsuucat wrote:
> 
> > You probably need to sign the copyright assignment form before any
> > further patches can be applied.
> 
> I have already assigned copyright for GNU Emacs.

Thanks.

> > IIRC your patch fixed a visual flicker rather than any actual
> > usability problem, is that right?
> 
> 
> Yes.

OK, I want to revert the patch on the Emacs 27 branch. This means the
flicker will be on Emacs 27, but at least it will be usable.

Please squash your three new patches together and have a look at the
instructions in CONTRIBUTE regarding commit messages.

Once you’ve done that I’ll push them to the master branch. Your
original patch will still be on master, so there’s no need to include
it.

Thanks for working on this and sorry again for the delay.
-- 
Alan Third

Message #29 received at 38851 <at> debbugs.gnu.org (full text, mbox):

From: tsuucat <tsuucat <at> icloud.com>
To: Alan Third <alan <at> idiocy.org>
Cc: 38851 <at> debbugs.gnu.org
Subject: Re: bug#38851: 27.0.50; Recent my patch breaks isearch with macOS
 native input method
Date: Fri, 14 Feb 2020 04:17:29 +0900

[Message part 1 (text/plain, inline)]

> Please squash your three new patches together and have a look at the
> instructions in CONTRIBUTE regarding commit messages.

OK. I squashed patches and attach the patch. Please tell me if there’s 
something wrong.

[0001-Fix-working-text-related-issues-on-NS-Bug-38851.patch (application/octet-stream, attachment)]

[Message part 3 (text/plain, inline)]

> Thanks for working on this and sorry again for the delay.

Thank you too!

--
tsuucat

Message #34 received at 38851-done <at> debbugs.gnu.org (full text, mbox):

From: Alan Third <alan <at> idiocy.org>
To: tsuucat <tsuucat <at> icloud.com>
Cc: 38851-done <at> debbugs.gnu.org
Subject: Re: bug#38851: 27.0.50; Recent my patch breaks isearch with macOS
 native input method
Date: Wed, 19 Feb 2020 11:14:20 +0000

On Fri, Feb 14, 2020 at 04:17:29AM +0900, tsuucat wrote:
> 
> > Please squash your three new patches together and have a look at the
> > instructions in CONTRIBUTE regarding commit messages.
> 
> OK. I squashed patches and attach the patch. Please tell me if there’s 
> something wrong.

Everything looks good to me, thank you. I’ve pushed to master.
-- 
Alan Third

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.