* bug#38851: 27.0.50; Recent my patch breaks isearch with macOS native input method
@ 2020-01-01 18:25 tsuucat via Bug reports for GNU Emacs, the Swiss army knife of text editors
2020-01-03 20:29 ` Alan Third
0 siblings, 1 reply; 10+ messages in thread
From: tsuucat via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2020-01-01 18:25 UTC (permalink / raw)
To: 38851
[-- Attachment #1: Type: text/plain, Size: 630 bytes --]
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.
[-- Attachment #2: 0001-Change-redisplay-solution.patch --]
[-- Type: application/octet-stream, Size: 1756 bytes --]
From 9f337cfc6c5daea484b4e1d42d20e017e798cdee Mon Sep 17 00:00:00 2001
From: Masahiro Nakamura <tsuucat@icloud.com>
Date: Thu, 2 Jan 2020 01:21:40 +0900
Subject: [PATCH] Change redisplay solution
---
src/keyboard.c | 6 ++++++
src/nsterm.m | 8 ++++----
2 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/src/keyboard.c b/src/keyboard.c
index 7d3b024..30fd526 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -2896,6 +2896,12 @@ read_char (int commandflag, Lisp_Object map,
example banishing the mouse under mouse-avoidance-mode. */
timer_resume_idle ();
+#ifdef HAVE_NS
+ if (CONSP (c)
+ && (EQ (XCAR (c), intern ("ns-unput-working-text"))))
+ input_was_pending = input_pending;
+#endif
+
if (current_buffer != prev_buffer)
{
/* The command may have changed the keymaps. Pretend there
diff --git a/src/nsterm.m b/src/nsterm.m
index c5cc182..52a9830 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -6430,6 +6430,10 @@ - (void)insertText: (id)aString
if (!emacs_event)
return;
+ /* First, clear any working text. */
+ if (workingText != nil)
+ [self deleteWorkingText];
+
/* It might be preferable to use getCharacters:range: below,
cf. https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/CocoaPerformance/Articles/StringDrawing.html#//apple_ref/doc/uid/TP40001445-112378.
However, we probably can't use SAFE_NALLOCA here because it might
@@ -6458,10 +6462,6 @@ - (void)insertText: (id)aString
emacs_event->code = code;
EV_TRAILER ((id)nil);
}
-
- /* Last, clear any working text. */
- if (workingText != nil)
- [self deleteWorkingText];
}
--
2.21.0
[-- Attachment #3: Type: text/plain, Size: 11 bytes --]
--
tsuucat
^ permalink raw reply related [flat|nested] 10+ messages in thread* bug#38851: 27.0.50; Recent my patch breaks isearch with macOS native input method
2020-01-01 18:25 bug#38851: 27.0.50; Recent my patch breaks isearch with macOS native input method tsuucat via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2020-01-03 20:29 ` Alan Third
2020-02-10 17:08 ` tsuucat via Bug reports for GNU Emacs, the Swiss army knife of text editors
0 siblings, 1 reply; 10+ messages in thread
From: Alan Third @ 2020-01-03 20:29 UTC (permalink / raw)
To: tsuucat; +Cc: 38851
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#38851: 27.0.50; Recent my patch breaks isearch with macOS native input method
2020-01-03 20:29 ` Alan Third
@ 2020-02-10 17:08 ` tsuucat via Bug reports for GNU Emacs, the Swiss army knife of text editors
2020-02-10 21:44 ` Alan Third
0 siblings, 1 reply; 10+ messages in thread
From: tsuucat via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2020-02-10 17:08 UTC (permalink / raw)
To: Alan Third; +Cc: 38851
[-- Attachment #1: Type: text/plain, Size: 1442 bytes --]
> 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
[-- Attachment #2.1: Type: text/html, Size: 2263 bytes --]
[-- Attachment #2.2: 0001-proper-selection-window-position-for-isearch.patch --]
[-- Type: application/octet-stream, Size: 1166 bytes --]
From db2b26d0891e60d67cad68982af44d759b407a58 Mon Sep 17 00:00:00 2001
From: Masahiro Nakamura <tsuucat@icloud.com>
Date: Sat, 4 Jan 2020 20:19:42 +0900
Subject: [PATCH] proper selection window position for isearch
---
src/nsterm.m | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/nsterm.m b/src/nsterm.m
index 52a9830..7674fb0 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -6557,13 +6557,18 @@ - (NSRect)firstRectForCharacterRange: (NSRange)theRange
{
NSRect rect;
NSPoint pt;
- struct window *win = XWINDOW (FRAME_SELECTED_WINDOW (emacsframe));
+ struct window *win;
NSTRACE ("[EmacsView firstRectForCharacterRange:]");
if (NS_KEYLOG)
NSLog (@"firstRectForCharRange request");
+ if (WINDOWP (echo_area_window) && ! NILP (call0 (intern ("ns-in-echo-area"))))
+ win = XWINDOW (echo_area_window);
+ else
+ win = XWINDOW (FRAME_SELECTED_WINDOW (emacsframe));
+
rect.size.width = theRange.length * FRAME_COLUMN_WIDTH (emacsframe);
rect.size.height = FRAME_LINE_HEIGHT (emacsframe);
pt.x = WINDOW_TEXT_TO_FRAME_PIXEL_X (win, win->phys_cursor.x);
--
2.21.0
[-- Attachment #2.3: Type: text/html, Size: 494 bytes --]
[-- Attachment #2.4: 0001-Remove-unfavorable-deleteWorkingText.patch --]
[-- Type: application/octet-stream, Size: 834 bytes --]
From 3a5c22dfc70197cce8cc42680d500a56ba8146b3 Mon Sep 17 00:00:00 2001
From: Masahiro Nakamura <tsuucat@icloud.com>
Date: Tue, 11 Feb 2020 01:51:14 +0900
Subject: [PATCH] Remove unfavorable deleteWorkingText
---
src/nsterm.m | 3 ---
1 file changed, 3 deletions(-)
diff --git a/src/nsterm.m b/src/nsterm.m
index 7674fb0..c7450b4 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -6675,8 +6675,6 @@ - (void)mouseDown: (NSEvent *)theEvent
NSTRACE ("[EmacsView mouseDown:]");
- [self deleteWorkingText];
-
if (!emacs_event)
return;
@@ -7301,7 +7299,6 @@ - (void)windowDidResignKey: (NSNotification *)notification
if (emacs_event && is_focus_frame)
{
- [self deleteWorkingText];
emacs_event->kind = FOCUS_OUT_EVENT;
EV_TRAILER ((id)nil);
}
--
2.21.0
[-- Attachment #2.5: Type: text/html, Size: 314 bytes --]
^ permalink raw reply related [flat|nested] 10+ messages in thread* bug#38851: 27.0.50; Recent my patch breaks isearch with macOS native input method
2020-02-10 17:08 ` tsuucat via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2020-02-10 21:44 ` Alan Third
2020-02-10 22:21 ` tsuucat via Bug reports for GNU Emacs, the Swiss army knife of text editors
2020-02-11 17:05 ` Eli Zaretskii
0 siblings, 2 replies; 10+ messages in thread
From: Alan Third @ 2020-02-10 21:44 UTC (permalink / raw)
To: tsuucat; +Cc: 38851
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
^ permalink raw reply [flat|nested] 10+ messages in thread* bug#38851: 27.0.50; Recent my patch breaks isearch with macOS native input method
2020-02-10 21:44 ` Alan Third
@ 2020-02-10 22:21 ` tsuucat via Bug reports for GNU Emacs, the Swiss army knife of text editors
2020-02-11 15:56 ` Eli Zaretskii
2020-02-11 17:22 ` Alan Third
2020-02-11 17:05 ` Eli Zaretskii
1 sibling, 2 replies; 10+ messages in thread
From: tsuucat via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2020-02-10 22:21 UTC (permalink / raw)
To: Alan Third; +Cc: 38851
> 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#38851: 27.0.50; Recent my patch breaks isearch with macOS native input method
2020-02-10 22:21 ` tsuucat via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2020-02-11 15:56 ` Eli Zaretskii
2020-02-11 17:22 ` Alan Third
1 sibling, 0 replies; 10+ messages in thread
From: Eli Zaretskii @ 2020-02-11 15:56 UTC (permalink / raw)
To: tsuucat; +Cc: alan, 38851
> 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@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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#38851: 27.0.50; Recent my patch breaks isearch with macOS native input method
2020-02-10 22:21 ` tsuucat via Bug reports for GNU Emacs, the Swiss army knife of text editors
2020-02-11 15:56 ` Eli Zaretskii
@ 2020-02-11 17:22 ` Alan Third
2020-02-13 19:17 ` tsuucat via Bug reports for GNU Emacs, the Swiss army knife of text editors
1 sibling, 1 reply; 10+ messages in thread
From: Alan Third @ 2020-02-11 17:22 UTC (permalink / raw)
To: tsuucat; +Cc: 38851
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#38851: 27.0.50; Recent my patch breaks isearch with macOS native input method
2020-02-11 17:22 ` Alan Third
@ 2020-02-13 19:17 ` tsuucat via Bug reports for GNU Emacs, the Swiss army knife of text editors
2020-02-19 11:14 ` Alan Third
0 siblings, 1 reply; 10+ messages in thread
From: tsuucat via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2020-02-13 19:17 UTC (permalink / raw)
To: Alan Third; +Cc: 38851
[-- Attachment #1: Type: text/plain, Size: 226 bytes --]
> 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.
[-- Attachment #2: 0001-Fix-working-text-related-issues-on-NS-Bug-38851.patch --]
[-- Type: application/octet-stream, Size: 3316 bytes --]
From 01250b154fc19c7dc22d70907f3089ad4b43244d Mon Sep 17 00:00:00 2001
From: Masahiro Nakamura <tsuucat@icloud.com>
Date: Fri, 14 Feb 2020 03:21:15 +0900
Subject: [PATCH] Fix working text related issues on NS (Bug#38851)
* src/keyboard.c (read_char): Prevent redsiplay right after
ns-unput-working-text event.
* src/nsterm.m ([EmacsView insertText:]): Partially revert commit
ba04217.
([EmacsView firstRectForCharacterRange:]): Fix candidate window
position when cursor is on echoarea.
([EmacsView mouseDown:])
([EmacsView windowDidResignKey:]): Don't delete working text.
---
src/keyboard.c | 6 ++++++
src/nsterm.m | 18 ++++++++++--------
2 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/src/keyboard.c b/src/keyboard.c
index bf1f5da..9dd7e00 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -2901,6 +2901,12 @@ read_char (int commandflag, Lisp_Object map,
example banishing the mouse under mouse-avoidance-mode. */
timer_resume_idle ();
+#ifdef HAVE_NS
+ if (CONSP (c)
+ && (EQ (XCAR (c), intern ("ns-unput-working-text"))))
+ input_was_pending = input_pending;
+#endif
+
if (current_buffer != prev_buffer)
{
/* The command may have changed the keymaps. Pretend there
diff --git a/src/nsterm.m b/src/nsterm.m
index 9d427b9..fddd4b0 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -6473,6 +6473,10 @@ - (void)insertText: (id)aString
if (!emacs_event)
return;
+ /* First, clear any working text. */
+ if (workingText != nil)
+ [self deleteWorkingText];
+
/* It might be preferable to use getCharacters:range: below,
cf. https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/CocoaPerformance/Articles/StringDrawing.html#//apple_ref/doc/uid/TP40001445-112378.
However, we probably can't use SAFE_NALLOCA here because it might
@@ -6501,10 +6505,6 @@ - (void)insertText: (id)aString
emacs_event->code = code;
EV_TRAILER ((id)nil);
}
-
- /* Last, clear any working text. */
- if (workingText != nil)
- [self deleteWorkingText];
}
@@ -6600,13 +6600,18 @@ - (NSRect)firstRectForCharacterRange: (NSRange)theRange
{
NSRect rect;
NSPoint pt;
- struct window *win = XWINDOW (FRAME_SELECTED_WINDOW (emacsframe));
+ struct window *win;
NSTRACE ("[EmacsView firstRectForCharacterRange:]");
if (NS_KEYLOG)
NSLog (@"firstRectForCharRange request");
+ if (WINDOWP (echo_area_window) && ! NILP (call0 (intern ("ns-in-echo-area"))))
+ win = XWINDOW (echo_area_window);
+ else
+ win = XWINDOW (FRAME_SELECTED_WINDOW (emacsframe));
+
rect.size.width = theRange.length * FRAME_COLUMN_WIDTH (emacsframe);
rect.size.height = FRAME_LINE_HEIGHT (emacsframe);
pt.x = WINDOW_TEXT_TO_FRAME_PIXEL_X (win, win->phys_cursor.x);
@@ -6713,8 +6718,6 @@ - (void)mouseDown: (NSEvent *)theEvent
NSTRACE ("[EmacsView mouseDown:]");
- [self deleteWorkingText];
-
if (!emacs_event)
return;
@@ -7341,7 +7344,6 @@ - (void)windowDidResignKey: (NSNotification *)notification
if (emacs_event && is_focus_frame)
{
- [self deleteWorkingText];
emacs_event->kind = FOCUS_OUT_EVENT;
EV_TRAILER ((id)nil);
}
--
2.21.0
[-- Attachment #3: Type: text/plain, Size: 88 bytes --]
> Thanks for working on this and sorry again for the delay.
Thank you too!
--
tsuucat
^ permalink raw reply related [flat|nested] 10+ messages in thread
* bug#38851: 27.0.50; Recent my patch breaks isearch with macOS native input method
2020-02-10 21:44 ` Alan Third
2020-02-10 22:21 ` tsuucat via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2020-02-11 17:05 ` Eli Zaretskii
1 sibling, 0 replies; 10+ messages in thread
From: Eli Zaretskii @ 2020-02-11 17:05 UTC (permalink / raw)
To: Alan Third; +Cc: 38851, tsuucat
> Date: Mon, 10 Feb 2020 21:44:51 +0000
> From: Alan Third <alan@idiocy.org>
> Cc: 38851@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.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-02-19 11:14 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-01-01 18:25 bug#38851: 27.0.50; Recent my patch breaks isearch with macOS native input method tsuucat via Bug reports for GNU Emacs, the Swiss army knife of text editors
2020-01-03 20:29 ` Alan Third
2020-02-10 17:08 ` tsuucat via Bug reports for GNU Emacs, the Swiss army knife of text editors
2020-02-10 21:44 ` Alan Third
2020-02-10 22:21 ` tsuucat via Bug reports for GNU Emacs, the Swiss army knife of text editors
2020-02-11 15:56 ` Eli Zaretskii
2020-02-11 17:22 ` Alan Third
2020-02-13 19:17 ` tsuucat via Bug reports for GNU Emacs, the Swiss army knife of text editors
2020-02-19 11:14 ` Alan Third
2020-02-11 17:05 ` Eli Zaretskii
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).