Issue 720: Spanish Keyboard Layout Accents #721

jgehrig · 2020-07-09T17:38:32Z

Issue #720: The [ character is not handled properly on Spanish Keyboard Layouts.

The Spanish Layout contains keys like ` and &. These keys add an accent to the next character typed.

For example:

QKeyEvent ev: QKeyEvent(KeyPress, Key_QuoteLeft, ShiftModifier, text="^") // Ignored, add accent to next key
QKeyEvent ev: QKeyEvent(KeyPress, Key_E, text="è")

Supporting this input mode will be interesting and may not be possible without adding state to NeovimQt::Input::convertKey.

codecov-io · 2020-10-17T04:50:58Z

Codecov Report

Merging #721 into master will increase coverage by 0.10%.
The diff coverage is 91.30%.

@@            Coverage Diff             @@
##           master     #721      +/-   ##
==========================================
+ Coverage   21.17%   21.28%   +0.10%     
==========================================
  Files          73       73              
  Lines       28137    28181      +44     
==========================================
+ Hits         5958     5998      +40     
- Misses      22179    22183       +4

Impacted Files	Coverage Δ
src/gui/shell.cpp	`43.98% <0.00%> (-0.09%)`	⬇️
src/gui/input.cpp	`91.46% <87.50%> (-0.54%)`	⬇️
test/tst_input_common.cpp	`100.00% <100.00%> (ø)`
test/tst_input_mac.cpp	`100.00% <100.00%> (ø)`
test/tst_input_unix.cpp	`100.00% <100.00%> (ø)`
test/tst_input_win32.cpp	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 83b39c3...6218900. Read the comment docs.

jgehrig · 2020-10-17T05:17:18Z

@alfredobarroso

Sorry for the huge delay here. Is there any chance you could do some testing with the current iteration?

I am not familiar with these keyboard layouts... It is working to the best of my knowledge (and my knowledge is not much haha). Each OS has slightly different behavior here. It would be great to get a sanity check from someone familiar with these keys.

Could you pay explicit attention to [ and ] on MacOS?

I observed the following non-working key events:

neovim-qt/test/tst_input_common.cpp

Lines 291 to 297 in 6218900

    
           //	// MacOS Alt + `: Prints [ 
        
           //	QKeyEvent evAltLeftSquareBracketMacOS{ QKeyEvent::KeyPress, Qt::Key_Less, Qt::AltModifier, QStringLiteral("[") }; 
        
           //	QCOMPARE(NeovimQt::Input::convertKey(evAltLeftSquareBracketMacOS), QStringLiteral("[")); 
        
           // 
        
           //	// MacOS Alt + \: Prints [ 
        
           //	QKeyEvent evAltRightSquareBracketMacOS{ QKeyEvent::KeyPress, Qt::Key_Apostrophe, Qt::AltModifier, QStringLiteral("]") }; 
        
           //	QCOMPARE(NeovimQt::Input::convertKey(evAltRightSquareBracketMacOS), QStringLiteral("["));

However, this could just be a layout mis-configuration... Linux/Windows behave differently from my MacOS Spanish layout.

alfredobarroso · 2020-10-23T12:02:37Z

@alfredobarroso

Sorry for the huge delay here. Is there any chance you could do some testing with the current iteration?

I am not familiar with these keyboard layouts... It is working to the best of my knowledge (and my knowledge is not much haha). Each OS has slightly different behavior here. It would be great to get a sanity check from someone familiar with these keys.

Could you pay explicit attention to [ and ] on MacOS?

I observed the following non-working key events:

neovim-qt/test/tst_input_common.cpp

Lines 291 to 297 in 6218900

// // MacOS Alt + `: Prints [

// QKeyEvent evAltLeftSquareBracketMacOS{ QKeyEvent::KeyPress, Qt::Key_Less, Qt::AltModifier, QStringLiteral("[") };

// QCOMPARE(NeovimQt::Input::convertKey(evAltLeftSquareBracketMacOS), QStringLiteral("["));

//

// // MacOS Alt + \: Prints [

// QKeyEvent evAltRightSquareBracketMacOS{ QKeyEvent::KeyPress, Qt::Key_Apostrophe, Qt::AltModifier, QStringLiteral("]") };

// QCOMPARE(NeovimQt::Input::convertKey(evAltRightSquareBracketMacOS), QStringLiteral("["));

However, this could just be a layout mis-configuration... Linux/Windows behave differently from my MacOS Spanish layout.

Sadly I don't have access to my Macbook. My country has limited mobility due to COVID so until the restrictions are lifted I can't pick it up :(

jgehrig · 2020-10-23T15:32:17Z

Sorry to hear that :(

I can relate. Here in the Northeastern States we were hit hard early and had aggressive shutdowns. No fun, strange times...

Let me know when you're able to test. The changes are ready for check-in pending validation. Any additional patching will be quick and easy.

jgehrig · 2021-04-14T18:22:23Z

@alfredobarroso

Friendly ping. I hope the COVID situation has improved for you!

codecov-commenter · 2021-05-31T06:26:14Z

Codecov Report

Merging #721 (bfae2aa) into master (1151c6d) will decrease coverage by 0.65%.
The diff coverage is 93.05%.

❗ Current head bfae2aa differs from pull request most recent head cae0636. Consider uploading reports for the commit cae0636 to get more accurate results

@@            Coverage Diff             @@
##           master     #721      +/-   ##
==========================================
- Coverage   22.75%   22.09%   -0.66%     
==========================================
  Files          81       74       -7     
  Lines       29031    28385     -646     
==========================================
- Hits         6606     6273     -333     
+ Misses      22425    22112     -313

Impacted Files	Coverage Δ
src/gui/shell.cpp	`44.96% <0.00%> (-6.97%)`	⬇️
src/gui/input.cpp	`91.71% <82.35%> (-1.41%)`	⬇️
src/gui/input_mac.cpp	`87.80% <100.00%> (+0.30%)`	⬆️
test/tst_input_mac.cpp	`100.00% <100.00%> (ø)`
test/tst_input_unix.cpp	`100.00% <100.00%> (ø)`
test/tst_input_win32.cpp	`100.00% <100.00%> (ø)`
src/gui/treeview.cpp	`44.18% <0.00%> (-17.72%)`	⬇️
src/gui/shelloptions.h	`87.50% <0.00%> (-12.50%)`	⬇️
src/gui/scrollbar.cpp	`58.33% <0.00%> (-7.44%)`	⬇️
... and 38 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1151c6d...cae0636. Read the comment docs.

jgehrig · 2021-06-01T01:49:20Z

@alfredobarroso

Do you have the ability to test this on MacOS again?

This should be ready for merge, but I'm not familiar with the Spanish keyboard layout. It would be good to have a native user test this before I hit the merge button.

Thanks!

alfredobarroso · 2021-06-01T06:19:11Z

Hi @jgehrig

Sorry for the HUGE delay. I've been away from home for most of the year without any access to a Mac. Now I'm back at home for a few days; I'll test it this afternoon (about 12 hours from now) and let you know.

alfredobarroso · 2021-06-01T13:37:49Z

Hi @jgehrig

I've tested again and some of the key combinations that failed on my previous tests are now working, but other seems to still be broken:

Key Combination	Expected	Obtained	Result
Option + '+'	]	]	OK
Option + 'ç'	}	}	OK
Option + '1'	Pipe	Pipe	OK
Option + '2'	@	@	OK
Option + '6'	¬	¬	OK
Option + '`'	[	ESC	FAIL
Option + '´'	{	ESC	FAIL
Option + '3'	#	ESC	FAIL
Option + 'º'	\	ESC	FAIL

'ESC' means that it behaves like pressing escape; it puts you into normal mode.

I got weird messages when compiling so I'm not 100% sure this is not a problem with my compilation.

I'm taking the Mac this time so count on me for any retests.

jgehrig · 2021-06-02T14:37:31Z

@alfredobarroso

Excellent. Thank you very much!

That table is very helpful. One quick ask: can you add a QKeyEvent column?

I just modified the PR to include some additional diagnostics for Debug builds (-DCMAKE_BUILD_TYPE=Debug).

Additional Table Column:

Key Combination	Expected	Obtained	Result	QKeyEvent
Option + '+'	]	]	OK	???
...	...	...	...	...

The QKeyEvent info is helpful both in fixing the FAIL cases and in adding test coverage for all cases.

Alternatively, if it is too hard to pick out the specific events, you can copy/pipe the output to a Logs.txt file and upload it.

'ESC' means that it behaves like pressing escape; it puts you into normal mode.

With the new diagnostics, it should be easier to identify the specific key; Very likely <Esc>.

I got weird messages when compiling so I'm not 100% sure this is not a problem with my compilation.

Let's see the build logs, just to be extra cautious. You should be able to drag/drop the log file into your comment.

Can you copy-paste the logs into a *.txt file?

I'm taking the Mac this time so count on me for any retests.

Awesome! Thanks 👍

Hopefully we can get this fixed quickly.

alfredobarroso · 2021-06-02T18:06:19Z

Here's the table with the QKeyEvent data:

Key Combination	Expected	Obtained	Result	QKeyEvent
Option + '+'	]	]	OK	QKeyEvent ev: QKeyEvent(KeyPress, Key_Plus, AltModifier, text="]") "]"
Option + 'ç'	}	}	OK	QKeyEvent ev: QKeyEvent(KeyPress, Key_Ccedilla, AltModifier, text="}") "}"
Option + '1'	\|	\|	OK	QKeyEvent ev: QKeyEvent(KeyPress, Key_1, AltModifier, text="\|") "\|"
Option + '2'	@	@	OK	QKeyEvent ev: QKeyEvent(KeyPress, Key_2, AltModifier, text="@") "@"
Option + '6'	¬	¬	OK	QKeyEvent ev: QKeyEvent(KeyPress, Key_6, AltModifier, text="¬") "¬"
Option + '`'	[	ESC	FAIL	QKeyEvent ev: QKeyEvent(KeyPress, Key_BracketLeft, AltModifier, text="[") "<A-[>"
Option + '´'	{	ESC	FAIL	QKeyEvent ev: QKeyEvent(KeyPress, Key_BraceLeft, AltModifier, text="{") "<A-{>"
Option + '3'	#	ESC	FAIL	QKeyEvent ev: QKeyEvent(KeyPress, Key_3, AltModifier, text="#") "<A-#>"
Option + 'º'	\	ESC	FAIL	QKeyEvent ev: QKeyEvent(KeyPress, Key_masculine, AltModifier, text="\\") "<A-Bslash>"

The build log looks good but I'm attaching it just in case. I think yesterday I mistook the brew install log for the build log.

build-log.txt

jgehrig · 2021-06-03T14:13:01Z

I wish Qt had a better mechanism for detecting keyboard layout/localization...

The remaining issues are AltModifier related, and very similar to #837 and #833. The [ { and \ characters will be tricky. The QKeyEvents are indistinguishable from valid sequences on other layouts.

We'll need to carefully pick behavior that minimizes negative impact across layouts. Probably disallow <A-{> and similar on MacOS?

alfredobarroso · 2021-06-03T14:41:08Z

Maybe, but I'm not sure if that would break other layouts :(

jgehrig · 2021-06-03T14:52:52Z

Maybe, but I'm not sure if that would break other layouts :(

It will (to some degree).

My Alt + [ (US Layout) is identical to yours. We'll probably just disable Alt + {}[] for everyone. This is probably an acceptable trade-off. Layouts other than Spanish use Alt for []{} and similar characters.

Accent characters are not handled correctly for the Spanish Keyboard Layout. TODO Adding tests only, not sure if these are correct... TODO Fix For Windows. Validate MacOS/Linux

jgehrig mentioned this pull request Sep 1, 2020

Accent keys with space work differently than expected #522

Closed

jgehrig force-pushed the jg-issue720 branch from 3f61273 to 8b44af0 Compare October 2, 2020 21:40

This comment has been minimized.

Sign in to view

jgehrig force-pushed the jg-issue720 branch from df711f0 to 48018d3 Compare October 17, 2020 04:07

jgehrig force-pushed the jg-issue720 branch 2 times, most recently from f33d5a7 to 7487dee Compare May 31, 2021 06:26

jgehrig force-pushed the jg-issue720 branch from 125743d to 73aa9a3 Compare June 2, 2021 15:41

jgehrig mentioned this pull request Jul 19, 2021

Left square bracket ([) not registered with Spanish keyboard #720

Open

jgehrig added 3 commits March 24, 2022 01:24

Issue 720: Spanish Keyboard Layout Accents

ec6198c

Accent characters are not handled correctly for the Spanish Keyboard Layout. TODO Adding tests only, not sure if these are correct... TODO Fix For Windows. Validate MacOS/Linux

DROP: QKeyEvent Debug Info

b34b76c

FIXME

cae0636

jgehrig force-pushed the jg-issue720 branch from bfae2aa to cae0636 Compare March 24, 2022 05:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue 720: Spanish Keyboard Layout Accents #721

Issue 720: Spanish Keyboard Layout Accents #721

jgehrig commented Jul 9, 2020

This comment has been minimized.

codecov-io commented Oct 17, 2020 •

edited

Loading

jgehrig commented Oct 17, 2020

alfredobarroso commented Oct 23, 2020

jgehrig commented Oct 23, 2020

jgehrig commented Apr 14, 2021

codecov-commenter commented May 31, 2021 •

edited

Loading

jgehrig commented Jun 1, 2021

alfredobarroso commented Jun 1, 2021

alfredobarroso commented Jun 1, 2021

jgehrig commented Jun 2, 2021

alfredobarroso commented Jun 2, 2021

jgehrig commented Jun 3, 2021

alfredobarroso commented Jun 3, 2021

jgehrig commented Jun 3, 2021

Issue 720: Spanish Keyboard Layout Accents #721

Are you sure you want to change the base?

Issue 720: Spanish Keyboard Layout Accents #721

Conversation

jgehrig commented Jul 9, 2020

This comment has been minimized.

codecov-io commented Oct 17, 2020 • edited Loading

Codecov Report

jgehrig commented Oct 17, 2020

alfredobarroso commented Oct 23, 2020

jgehrig commented Oct 23, 2020

jgehrig commented Apr 14, 2021

codecov-commenter commented May 31, 2021 • edited Loading

Codecov Report

jgehrig commented Jun 1, 2021

alfredobarroso commented Jun 1, 2021

alfredobarroso commented Jun 1, 2021

jgehrig commented Jun 2, 2021

alfredobarroso commented Jun 2, 2021

jgehrig commented Jun 3, 2021

alfredobarroso commented Jun 3, 2021

jgehrig commented Jun 3, 2021

codecov-io commented Oct 17, 2020 •

edited

Loading

codecov-commenter commented May 31, 2021 •

edited

Loading