CMD arguments and CaptureMode was mixed up
Needs RevisionPublic

Authored by aprcela on Sep 11 2019, 10:36 PM.

Diff Detail

Repository
R166 Spectacle
Branch
B411290
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 16410
Build 16428: arc lint + arc unit
aprcela created this revision.Sep 11 2019, 10:36 PM
Restricted Application added a project: Spectacle. · View Herald TranscriptSep 11 2019, 10:36 PM
Restricted Application added a subscriber: Spectacle. · View Herald Transcript
aprcela requested review of this revision.Sep 11 2019, 10:36 PM
ngraham accepted this revision.Sep 11 2019, 11:26 PM
ngraham added a subscriber: ngraham.

Whoops.

This revision is now accepted and ready to land.Sep 11 2019, 11:26 PM
This revision was automatically updated to reflect the committed changes.
davidre reopened this revision.EditedSep 12 2019, 8:42 AM
davidre added a subscriber: davidre.

Did you test this? I think the way it was was correct. A transient window is for example a popup . Only the popup is transientOnly (capturemode window under cursor) and everything is transientwithparent. See also the help text

-u, --windowundercursor    Capture the window currently under the cursor,
                             including parents of pop-up menus
  -t, --transientonly        Capture the window currently under the cursor,
                             excluding parents of pop-up menus

and the gui version
https://cgit.kde.org/spectacle.git/tree/src/Gui/KSWidget.cpp#n217

Please revert this

This revision is now accepted and ready to land.Sep 12 2019, 8:42 AM
davidre requested changes to this revision.Sep 12 2019, 8:42 AM
This revision now requires changes to proceed.Sep 12 2019, 8:42 AM

Did you test this? I think the way it was was correct. A transient window is for example a popup . Only the popup is transientOnly (capturemode window under cursor) and everything is transientwithparent. See also the help text

-u, --windowundercursor    Capture the window currently under the cursor,
                             including parents of pop-up menus
  -t, --transientonly        Capture the window currently under the cursor,
                             excluding parents of pop-up menus

and the gui version
https://cgit.kde.org/spectacle.git/tree/src/Gui/KSWidget.cpp#n217

Please revert this

Damn, looks like I mixed up while testing with the gui for Bug 411290.. Yeah, the way it was, is correct..
Should I revert or can ngraham undo the commit?

Will revert.

We should consider renaming the enums or serialized values or adding a comment in the code though, because the way this is written is is confusing and looks wrong (even if it's right).

Will revert.

We should consider renaming the enums or serialized values or adding a comment in the code though, because the way this is written is is confusing and looks wrong (even if it's right).

I don't think it was confusing in this instance there is a difference between transientonly and TransientWithParent. From the name I expect them to be different things