Thanks for the patch! I had not noticed the problem before :)

I'm starting to think that we need an option for enabling/disabling this change/feature. I would not want to have the extra space between the lines, but at the same time I can see that actually not seeing the whole character is not an acceptable situation...

I would like to use "this" as parent for the QShortcuts in stead of the parent of KSaneWidget that actually might be nullptr.

It seems it could be quite hard to get this to work with only the QPushButton, so your idea with a separate QShortcut is a good workaround.

This seems to be the reason...
https://mail.kde.org/pipermail/plasma-devel/2016-May/052732.html

@ltoscano Yes sure, I have not made a release in over two years :( so waiting a bit more should not be a problem ;)

In D28672#661177, @ngraham wrote:

Also @sars, I notice that Skanlite hasn't gotten a release in a while. I wonder if it would be beneficial to move it to the release service so that it can share the same release schedule as most KDE apps and automatically get three major releases per year, with three minor releases per major one. What do you think?

That should work, but I can confirm that it does not work in libksane at the moment.

Yes please add that to libksane :)

Sorry for the faaaar to late response...

I would like to eventually have the "ctrl-..." along sides the auto generated shortcuts. This is basically a workaround to gett a shortcut (hopefully "Alt-s") for the scan button in stead of no shortcut at the moment.

I'm not a big fan of the assigning of shortcuts with &, because it becomes a big hassle to keep track of what has been set where and translations makes things even more complicated...

Both Konsole and Kate, take the default font from the global fixed with font settings. Once you change the setting in the application, it no-longer follows the global setting. I have also noticed that the default font settings seem to be a bit distro specific.

Thank you both for the reviews :)

But I'm fine without the changes also :)

• Fix inch as display unit: 1 inch = 25.4 mm (it was not cm->inch)

Everything is big in America :)

Everything is big in America ;) (Legal should be 8.5 X 14 inch)

@antonarnold I'm not exactly sure what is wrong in those ;)

@antonarnold Good bug catch! All the scan sources I have here, have the same size so I did never reach that code path...

• Set the page size to "Custom" when the source size changes.

Ah OK :). The spinbox+slider is a reused widget that is used for all numeric properties in libksane :)

The sliders provide the size of the scanned area in numeric form and there is the possibility to fine-tune the scan area without having to be too steady on the hand when doing the selection. So I do see a value in having the sliders in some form...

I'm not 100% happy with having the size sliders there directly after the color options.

Do you want to send a new version or should I Just remove the separator removal and use "... = QLocale::toFloat(tmp);" and commit in your name?

Thanks for these fixes :)

Yes it works :)

I have been a bit reluctant to add the predefined page sizes, but maybe it is time to rethink it...

I would agree that a UNIX/POSIX env var is not the most logical thing to use on Windows, but how do we ensure that we are in sync with ki18n? The usage of QLocale in ki18n looks like it is just adding extra languages to a list of languages, not actually using QLocale to determine what catalog to load. At least LANGUAGE overrides any QLocale settings. Should ki18n not be moved to using QLocale and setDefault()?

I was actually wondering why the F4 shortcut was put to Focus/Defocus Terminal and not to the "Show Terminal" action. The later action works the same as Dolphin (except that the focus to the terminal got broken at some point this fall or summer).

So the conclusion is that we are not using setDefault() yet and ki18n & gettext uses the LANGUAGE env so the logical thing, for now, is to also use LANGUAGE env here in KCoreAddons for the plugin metadata translations?

ki18n does not seem to use QLocale to get the language for translation, but uses LANGUAGE env. Also gettext uses the env variable.

BTW should this also be done to to other options setting numbers?

Scratch that 19.12 comment, the 19.12 feature freeze was already the 14th... The bug fixes can go in as they are bug fixes ;) so no hurry.

That is absolutely fine!

I hope I did not scare you off with my comments. These fixes are really welcome!

Isn't the change in setOptVals() the only change needed for fixing the scan option order problem?

Generally good changes :)

Hmm yes, that order in which we restore the saved options could have a priority order. We could just check if the map contains scan source and execute that first. Then check for mode and resolution and then after that the rest.

Thanks for your review request! :)

@ngraham Unfortunately there is no way with UI files to dynamically modify the layout depending on the size of the container, so munging the layout in C++ is the best way I know so far :)

Sorry for the late review.

Imagine you have a "minimized" javascript or XML file. You basically have the whole file on one line. This can be thousands of characters on a line. If you now search for a fairly common word in the minimized code, you would duplicate the huge line as many times as you get matches.