Possible bug?

[KevinH]

@BeckyEbook,
When you get a free moment, please try with altgr in the command line with what I just pushed to master ...

Hopefully this change will make it somewhat more like Sigil-1.7 handling when altgr worked.

For those interested n what I added, here is the change just pushed to master to try to handle this case.

diff --git a/src/Dialogs/PreferenceWidgets/KeyboardShortcutsWidget.cpp b/src/Dialogs/PreferenceWidgets/KeyboardShortcutsWidget.cpp
index b7f77042..688189f6 100644
--- a/src/Dialogs/PreferenceWidgets/KeyboardShortcutsWidget.cpp
+++ b/src/Dialogs/PreferenceWidgets/KeyboardShortcutsWidget.cpp
@@ -276,6 +276,10 @@ void KeyboardShortcutsWidget::handleKeyEvent(QKeyEvent *event)
 
 
 #ifdef Q_OS_WIN32
+    if ((state & Qt::GroupSwitchModifier)) {
+        letter = "" + QChar(nextKey);
+        state = state & ~((int)Qt::GroupSwitchModifier);
+    }
     if ((state & Qt::ShiftModifier) && letter.toUpper() == letter.toLower()) {
         // remove the shift since it is included in the keycode
         state = state & ~Qt::SHIFT;

[BeckyEbook]

It's OK!
Now AltGr works as in 1.7.0 released, no garbage in the Shortcut input box.

The current changes do not require a workaround and manual correction of the sigil.ini file, and everything works. Though I'm only human, someone else should check it out.

[KevinH]

Wonderful!
Thank you for all your help on this!
I will clean up the code a bit to streamline it but leave in the qdebug stuff so that other people can test as well.

[KevinH]

Yes, I think we should use a preference setting to effectively add this to the command line somehow so that international windows users can take advantage of this much more easily.

Originally Posted by DiapDealer

Is that a command-line parameter for running Sigil? This is the first I've heard of international keyboard users needing to add special parameters for running Sigil. Would most users even know how? If it's necessary, then we should probably find a way to load it programmatically via a pref setting rather than relying on people stumbling across an obscure parameter to the qwindow platform plugin. Can you point me to some documentation on that platform plugin parameter? I can't seem to find anything.

Originally Posted by BeckyEbook

https://doc.qt.io/qt-5/qguiapplicati...GuiApplication

We talked about it here:
https://github.com/Sigil-Ebook/Sigil/issues/474

the parameter was mentioned e.g. here:
https://bugreports.qt.io/browse/QTBU...comment-504202

https://bugreports.qt.io/browse/QTBU...comment-416209

is also in Sigil's documentation:
https://github.com/Sigil-Ebook/sigil...faq.xhtml#L228

[DiapDealer]

I'm not seeing any way to easily set the parameter at runtime with a preference. I've not come across any equivalent Qt::AA_ or anything. You can pass platform plugin arguments via a qt.conf file, though:

[Platforms]
WindowsArguments=altgr

And we can include a qt.conf file as an internal resource, but I'm not certain we could include logic to load different qt.conf files (or skip loading one entirely). Or if we could, I don't know if it would get loaded soon enough to effect the windows platform plugin.

I'm just not certain how to proceed.

[DiapDealer]

If @BeckyEbook can verify the parameter can be set using the QT_QPA_PLATFORM environment variable, we might be able to inject it (if it's not already set to something else) in main.cpp.

Assigning the variable a windows:altgr value should tell us if its even possible or not.

[BeckyEbook]

Or (maybe easier) – add logic to the .iss file and ask the user during installation whether he uses AltGr (right Alt) to enter diacritics or other national characters. Only interested users will select the checkbox.

[BeckyEbook]

I confirm, that works!

set QT_QPA_PLATFORM=windows:altgr

However, the ability to add a parameter is also useful, so it's good to have both possibilities discovered.

[KevinH]

I looked at:

qtbase/src/plugins/platforms/windows/qwindowsintegration.cpp

And that is in the QPA platform plugin code and it is where that command line option is processed:

QWindowIntegration::DetectAltGrModifier but unless we use the QCommandLine processor and somehow in main.cpp before opening the QApplication we try and append "-platform windows:altgr" to the actual command line arguments, it might work but ... I am not sure even then it would be early enough.

Perhaps there is no easy way other than what the Sigil User's guide approach recommends.

[DiapDealer]

Originally Posted by BeckyEbook

I confirm, that works!

set QT_QPA_PLATFORM=windows:altgr

Great, thanks!

Originally Posted by BeckyEbook

However, the ability to add a parameter is also useful, so it's good to have both possibilities discovered.

The -platform command line option is supposed to override the QT_QPA_PLATFORM variable, so user customization should still be possible.

I'd like to avoid setting it up with InnoSetup options at install time if possible. I could probably finagle adding the command line option to the desktop shortcut that way, but I couldn't really take care of all the ways a user could potentially bypass the altgr parameter (even when they didn't want to bypass it) without doing a lot of registry meddling to make sure Sigil always ran with the option without fail.

[DiapDealer]

Originally Posted by KevinH

but ... I am not sure even then it would be early enough.

Perhaps there is no easy way other than what the Sigil User's guide approach recommends.

Just have to try, I guess. If it doesn't work, it doesn't work.

I've also been thinking about a simple C launcher for Sigil where things like environment variables, or command-line option could be easily set up before the real Sigil executable is run. Sort of like the launcher-script for Sigil on Linux, but compiled. I've got one for Windows somewhere that allows Sigil to be run with a portable prefs directory.

[KevinH]

Perhaps at line 362 in our main.cpp we could copy both argc and argv and increment our version of argc and add the needed option to the end of our version of argv before passing them to MainApplication. I think the app name has already been set so our SettingsStore may be usable at that point.

But just a guess.

[DiapDealer]

Originally Posted by KevinH

Perhaps at line 362 in our main.cpp we could copy both argc and argv and increment our version of argc and add the needed option to the end of our version of argv before passing them to MainApplication. I think the app name has already been set so our SettingsStore may be usable at that point.

But just a guess.

I'll take a look. The SettingsStore should definitely be usable by then. I use it back at line 339 to set up the highdpi stuff.

[KevinH]

My only fear is that windows does not use the normal argc, argv that macOS and Linux use since they get their arguments via an internal Windows (*W routine for wide chars).

Not sure if there is a Windows specific call to add additional arguments.

Edit:
It will probably not work given these comments in Qt source:

https://github.com/qt/qtbase/blob/b4...tion.cpp#L2412

[DiapDealer]

It seems to me that creating the QT_QPA_PLATFORM environment variable early enough in main.cpp just _might_ be working. It's just very hard for me to test without an actual international keyboard.

@BeckyEbook: can you try inserting the following line in main.cpp at line 363 (just before the MainApplication app(argc, argv); line):

qputenv("QT_QPA_PLATFORM", "windows:altgr");

And see if it works for you? Make sure you run Sigil without the -platform windows:altgr parameter.

I don't want to play around with ifdefing for Windows and creating new preference settings if it's not going to work.