Don’t hijack `SELF-INSERT-COMMAND`.

[ieure]

This PR reworks how input into Crossword grid is handled. Instead of adding before-advice to SELF-INSERT-COMMAND, then aborting before the underlying command can run, it binds all alphabetic keys to CROSSWORD-SELF-INSERT.

Additionally, it changes CROSSWORD--INSERT-CHAR to take an explicit CHAR argument. This allows CROSSWORD-DEL-CHAR and CROSSWORD-BSP-CHAR to avoid mutating LAST-INPUT-EVENT -- they pass the char as an argument instead.

Together, these changes remove the kludge mentioned in (the old) CROSSWORD--PRE-INSERT and solve the flashing "Quit" message in the echo area.

[Boruch-Baum]

Thanks, I'll take a look. I originally didn't do this for two reasons:

• I wanted the user to be able to insert arbitrary characters ans 'signs' or 'messages' to oneself for questionable clues. These could include unicode wing-dings or emojis or anything else, even simple !@#$%^&*().

• I wanted the code to be flexible to support puzzles written for any language and character set, eg. Hebrew, Russian.

-- hkp://keys.gnupg.net CA45 09B5 5351 7C11 A9D1 7286 0036 9E45 1595 8BC0

[ieure]

Thanks, I'll take a look. I originally didn't do this for two reasons:

• I wanted the user to be able to insert arbitrary characters ans 'signs' or 'messages' to oneself for questionable clues. These could include unicode wing-dings or emojis or anything else, even simple !@#$%^&*().

I'd suggest defining some concrete set of characters, and adding those to the CL-LOOP that binds CROSSWORD-SELF-INSERT. Or perhaps you could look at every key in GLOBAL-MAP which is bound to SELF-INSERT-COMMAND, and bind those to CROSSWORD-SELF-INSERT in the CROSSWORD-MODE-MAP.

• I wanted the code to be flexible to support puzzles written for any language and character set, eg. Hebrew, Russian.

Non-Latin input methods are a thing I'm not familiar with, but I suspect it's not a good idea to optimize for them right now. They're likely to have plenty of problems beyond the keybinding -- like doublewidth characters breaking the puzzle layout, or LTR languages rendering backwards.

[Boruch-Baum]

On 2021-01-19 22:47, Ian Eure wrote:

Non-Latin input methods are a thing I'm not familiar with, but I suspect it's not a good idea to optimize for them right now.

No need to optimize for most language cases. And the issue affects not just languages foreign to Europe: The code as written should properly support the unique characters of German, French, Spanish, Polish, Nordic languages.

They're likely to have plenty of problems beyond the keybinding -- like doublewidth characters breaking the puzzle layout

Multi-byte shouldn't be a problem at this point in the unicode adoption. And, for example, the code depends on multi-byte unicode support for properly lining up the block-squares.

LTR languages rendering backwards.

My feelings exactly! LTR languages are rendered backwards. Silly Europeans.

-- hkp://keys.gnupg.net CA45 09B5 5351 7C11 A9D1 7286 0036 9E45 1595 8BC0

[ieure]

On 2021-01-19 22:47, Ian Eure wrote: Non-Latin input methods are a thing I'm not familiar with, but I suspect it's not a good idea to optimize for them right now. No need to optimize for most language cases. And the issue affects not just languages foreign to Europe: The code as written should properly support the unique characters of German, French, Spanish, Polish, Nordic languages. They're likely to have plenty of problems beyond the keybinding -- like doublewidth characters breaking the puzzle layout Multi-byte shouldn't be a problem at this point in the unicode adoption. And, for example, the code depends on multi-byte unicode support for properly lining up the block-squares.

I'm not speaking of multibyte characters, but of those which display at a larger width than a latin character within an Emacs buffer.

I'm also noting that the way crossword.el stastics are computed depends on whether (string-match "[[:upper:]]" CHAR) is t. I guess this is supposed to be for supporting ?, ! etc, however, it shows that your two goals are in conflict: Many scripts (including Hebrew, which you mention) don't have upper/lower case. So anyone attempting to solve a puzzle in those languages will have problems either way. But merging my PR will solve major, concrete problems which all crossword.el users have today.

[Boruch-Baum]

On 2021-01-19 23:26, Ian Eure wrote:

Many scripts (including Hebrew, which you mention) don't have upper/lower case. ...

That would be a bug.

But merging my PR will solve major, concrete problems which all crossword.el users have today.

How major? Are you getting anything more than quit messages to the message buffer. That's important to know, because I hardly notice the issue, so that's important feedback because I had thought the issue was minor.

As written, the PR seems to break the program for use even in non-English European languages, right?

-- hkp://keys.gnupg.net CA45 09B5 5351 7C11 A9D1 7286 0036 9E45 1595 8BC0

[ieure]

On 2021-01-19 23:26, Ian Eure wrote: Many scripts (including Hebrew, which you mention) don't have upper/lower case. ... That would be a bug.

My point is that solving that bug causes a conflict with your other stated goals. So you have to make a decision which is more important. My position is that my PR gives you a better, more solid foundation to solve the other problems on.

But merging my PR will solve major, concrete problems which all crossword.el users have today.

How major? Are you getting anything more than quit messages to the message buffer. That's important to know, because I hardly notice the issue, so that's important feedback because I had thought the issue was minor.

You may feel it's an abstract issue, but hijacking the global SELF-INSERT-COMMAND is not a good practice. You risk breaking entire Emacs sessions if your code has a bug.

But concretely:

• My solution is less code.
• The code is simpler, less fragile, and less risky.
• It solves what you yourself call "a kludge."
• Calling KEYBOARD-QUIT stops the event loop, which causes key events to be dropped when you type fast. Example, if I type "ABCDEFG," only "ABE" is inserted; the other events are lost. My code fixes that.

As written, the PR seems to break the program for use even in non-English European languages, right?

As mentioned, it is already broken. Do you have a non-Latin script crossword puzzle to test the before/after against?

[Boruch-Baum]

On 2021-01-20 12:10, Ian Eure wrote:

You may feel it's an abstract issue, but hijacking the global SELF-INSERT-COMMAND is not a good practice. You risk breaking entire Emacs sessions if your code has a bug.

That's why the very first thing it does is check state to remove itself from a global context.

But concretely:

• My solution is less code.

Yes, but in a pre-unicode, pre-i18n, pre-l10n, 20th century style that's been frowned upon for decades.

 * The code is simpler, less fragile, and less risky.

It's rigidly ties to English-language locales.

 * It solves what you yourself call "a kludge."

Yes.

 * Calling KEYBOARD-QUIT stops the event loop, which causes key events
   to be dropped when you type fast. Example, if I type "ABCDEFG,"
   only "ABE" is inserted; the other events are lost. My code fixes
   that.

I can't reproduce that at my fastest typing speed, and don't see that as practical. At what typing speed does that happen to you?

 As written, the PR seems to break the program for use even in
 non-English European languages, right?

As mentioned, it is already broken.

No, it isn't. You just claimed that without providing basis.

Do you have a non-Latin script crossword puzzle to test the
before/after against?

No. That would be nice to get. Please share.

-- hkp://keys.gnupg.net CA45 09B5 5351 7C11 A9D1 7286 0036 9E45 1595 8BC0

[ieure]

Okay whatever. I've spent longer arguing with you than it took to fix the code. I'll just fork and do things how I like. Have a nice life.