Re: [gnome-boxes 1/2] Skip 'Preparation' & 'Setup' when going backwards

["Zeeshan Ali (Khattak)" <zeeshanak gnome org>]

On Thu, Nov 3, 2011 at 6:38 PM, Marc-André Lureau
<marcandre lureau gmail com> wrote:
> hi
>
> On Thu, Nov 3, 2011 at 5:25 PM, Zeeshan Ali (Khattak)
> <zeeshanak gnome org> wrote:
>> From: "Zeeshan Ali (Khattak)" <zeeshanak gnome org>
>>
>> Skip 'Preparation' and 'Setup' pages when going backwards in the wizard.
>> ---
>>  src/wizard.vala |   10 ++++++++--
>>  1 files changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/wizard.vala b/src/wizard.vala
>> index 6bae45c..8ceb21d 100644
>> --- a/src/wizard.vala
>> +++ b/src/wizard.vala
>> @@ -47,7 +47,8 @@ private class Boxes.Wizard: Boxes.UI {
>>
>>             case WizardPage.PREPARATION:
>>                 try {
>> -                    prepare ();
>> +                    if (value > page)
>> +                        prepare ();
>
> that's hard to read. perhaps it should be placed on top the function
> to skip also other cases.
> and use a variable backward = value <= page; for readibility.

  Done!

>>                 } catch (GLib.Error error) {
>>                     warning ("Fixme: %s".printf (error.message));
>>                     return;
>> @@ -250,10 +251,15 @@ private class Boxes.Wizard: Boxes.UI {
>>
>>     private bool skip_page (Boxes.WizardPage page) {
>>         // remote-display case
>> -        if (page > Boxes.WizardPage.SOURCE &&
>> +        if (page > this.page &&
>> +            page > Boxes.WizardPage.SOURCE &&
>>             page < Boxes.WizardPage.REVIEW &&
>>             this.source != null)
>>             return true;
>
> That condition was pretty clear to me, skip that range if we have a
> new source already:
>
>        if (Boxes.WizardPage.SOURCE < page < Boxes.WizardPage.REVIEW &&
>            this.source != null)
>            return true;
>
> you added page > this.page which means going forward, correct? it also
> needs to apply backward in this case (for source URI).

  When going backwards, we can be sure that we already have a URI
since there is no way to get past Preparation if you don't have URI.

>> +        else if (page < this.page &&
>> +                 page == Boxes.WizardPage.PREPARATION ||
>> +                 page == Boxes.WizardPage.SETUP)
>> +            return true;
>
> Which I translate to if going backward, always skip preparation &
> setup. This doesn't seem to be correct, as you may want to change your
> username in the Setup step.

  True but we are currently skipping Review when going forward so it
makes sense to skip this empty step in other direction too.


-- 
Regards,

Zeeshan Ali (Khattak)
FSF member#5124