Optimize support of scanners with different image sources or duplex unit
Needs ReviewPublic

Authored by antonarnold on Oct 29 2019, 6:10 PM.

Details

Reviewers
sars
Group Reviewers
KDE Applications
Summary

BUG: 414600

The initial changeset of D24904 has been split into two seperate revisions.
This one addresses various bugs which may result in inconsistent scanner options.

Diff Detail

Repository
R382 KSane Library
Lint
Lint Skipped
Unit
Unit Tests Skipped
antonarnold requested review of this revision.Oct 29 2019, 6:10 PM
antonarnold created this revision.

Can you re-add the BUG: keywords to the description for the bugs that this patch fixes? Thanks!

sars added a comment.Oct 29 2019, 7:14 PM

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

Looking forward to the next iteration :)

src/ksanewidget.cpp
106

no spaces in the signal/slot signatures

471

Is this a bug-fix or a mistake?

747

This could be simplified:

if (map.contains(foo)) {

if (setValue(foo, map[foo])) {
    ret++;
}
map.remove(foo);

}

936

looks like these are the same white-space changes as in the other review. This will lead to merge conflicts.

src/ksanewidget_p.cpp
1167

signal/slot signature without spaces...

src/options/ksaneoptcombo.cpp
334

maybe just remove the whole commented code and the comment.

352

This fix could have been a separate commit and review. (it has nothing to do with the reorganization of the restoration of the options)

antonarnold edited the summary of this revision. (Show Details)

reverted whitespace changes, fixed headers, filed and linked bug, moving seperate bugs to own diff

antonarnold added inline comments.Thu, Nov 28, 12:22 PM
src/ksanewidget.cpp
471

this is part of the bugfix that options become inconsistent

747

I assume you mean setOptValue? I'd rather avoid calling it since it does some additional stuff besides setting the option.

removed unneeded header include that I forget

sars added inline comments.Tue, Dec 3, 6:35 PM
src/ksanewidget.cpp
432

Why do we need to rearrange the m_optList? Isn't it enough that we restore the options in the right order?

748

I try to avoid assignments in the if() statement as it is harder to read and the risk of mistakes is higher. But this is a bit of taste issue ;)

src/ksanewidget_p.cpp
501

The other "Try to ..." statements try to change the default "Color mode", 8bits/color and 600DPI.

This reads the source and writes the same value back.... Why? Do you have a buggy back-end that somehow requires this? In that case I think it should be fixed in the back-end.

985

Good addition :)

src/options/ksaneoptcombo.cpp
352

Still holds (could be in the bug-fix commit), but we can let it in here anyways :D