Merge lp:~evfool/synaptic/lp975578 into lp:synaptic

Proposed by Robert Roth on 2012-11-12
Status: Merged
Merged at revision: 2105
Proposed branch: lp:~evfool/synaptic/lp975578
Merge into: lp:synaptic
Diff against target: 12 lines (+1/-1)
1 file modified
gtk/rgmainwindow.cc (+1/-1)
To merge this branch: bzr merge lp:~evfool/synaptic/lp975578
Reviewer Review Type Date Requested Status
Daniel Hartwig (community) Needs Fixing on 2012-11-13
synaptic-developers 2012-11-12 Pending
Review via email: mp+133934@code.launchpad.net

Description of the change

Do not show Successfully marked all available updates status text in case the user cancels the operation on the summary dialog. (LP: #975578)

To post a comment you must log in.
Daniel Hartwig (wigs) wrote :

On cancel, the status will be set to:
 Failed to mark all available upgrades!

which is not correct.

This should rather not-set/clear the status message, as per cancelling other operations (see gtk/rgmainwindow.cc:2905, for example), or at least change to mention “cancelled”. Below is a patch for this. Note that the final call to showErrors is a noop on cancel, since askStateChange will discard pending errors if the user cancels.

=== modified file 'gtk/rgmainwindow.cc'
--- gtk/rgmainwindow.cc 2012-07-12 20:03:08 +0000
+++ gtk/rgmainwindow.cc 2012-11-13 01:58:53 +0000
@@ -3247,10 +3247,12 @@
    else
       res = me->_lister->upgrade();

- me->askStateChange(state);
+ bool changed = me->askStateChange(state);
    me->refreshTable(pkg);

- if (res)
+ if (!changed)
+ me->setStatusText();
+ else if (res)
       me->setStatusText(_("Successfully marked available upgrades"));
    else
       me->setStatusText(_("Failed to mark all available upgrades!"));

review: Needs Fixing
Daniel Hartwig (wigs) wrote :

Formatting for the above patch (i.e. whitespace) not preserved by the comment system, but you get the idea.

Daniel Hartwig (wigs) wrote :

… and I see that mvo already fixed this in the merge, never mind then :-)

Michael Vogt (mvo) wrote :

On Tue, Nov 13, 2012 at 02:23:42AM -0000, Daniel Hartwig wrote:
> … and I see that mvo already fixed this in the merge, never mind then :-)

Thanks Robert for your fix! I merged it with the small tweak that
Daniel pointed out, i.e. not show a error if the user cancels.

@Daniel: I do like your approach using the "bool changed = " as it
makes more explicit what actually happend.

Cheers,
 Michael

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'gtk/rgmainwindow.cc'
2--- gtk/rgmainwindow.cc 2012-07-12 20:03:08 +0000
3+++ gtk/rgmainwindow.cc 2012-11-12 14:37:23 +0000
4@@ -3247,7 +3247,7 @@
5 else
6 res = me->_lister->upgrade();
7
8- me->askStateChange(state);
9+ res = res & me->askStateChange(state);
10 me->refreshTable(pkg);
11
12 if (res)

Subscribers

People subscribed via source and target branches

to status/vote changes: