Wireshark-dev: Re: [Wireshark-dev] Issues with extcap arguments type radio

From: Jaap Keuter <jaap.keuter@xxxxxxxxx>
Date: Sat, 6 Aug 2022 18:17:10 +0200
Hi Roland,

Thanks for confirming this. I’m already working on setDefaultValue, for radio button. Seems to be working.
I hope to push something later tonight.

Thanks,
Jaap

On 6 Aug 2022, at 14:01, Roland Knall <rknall@xxxxxxxxx> wrote:

Hi

Yes, takeAt seems to be a mistake, which is easily explained. Storing the values for the radios was not implemented initially, so it might have been overlooked by me at that time, that this was not working. 

For restoring, you can either adapt "setDefaultValue" to check for the stored value, or "createEditor" to set it to the restored value while creating the widget. Personally, I would adapt "setDefaultValue", as it is called after "createEditor" and therefore you avoid the setted index being overwritten.

Btw, an argument could be made for not storing the index but rather the value of the option field, due to the indices being able to change with an adapted extcap program. Personally I think, it is much more likely that the indices changed and the values remained the same, therefore this seems to be the more future-prone approach. But of course it might also be more error-prone, as you would need to make a string-compare to figure out which index to set. 

cheers
Roland


Am Sa., 6. Aug. 2022 um 13:50 Uhr schrieb Jaap Keuter <jaap.keuter@xxxxxxxxx>:
Hi Guy,

I’ll go with the idea of replacing takeAt() with at() in the ExtArgRadio::value method for now.
Next is to find out why the saved setting for the radio button is not being restored when reopening the extcap configuration dialog.

Thanks,
Jaap


> On 6 Aug 2022, at 12:55, Guy Harris <gharris@xxxxxxxxx> wrote:
>
> On Aug 6, 2022, at 2:55 AM, Jaap Keuter <jaap.keuter@xxxxxxxxx> wrote:
>
>> It uses QList::takeAt() to retrieve the value. But according to the documentation it does:
>>
>>     T t = at(i);
>>     remove(i);
>>     return t;
>>
>>
>> So it in fact removes the value from the list.
>
> Yes - the documentation for QList::takeAt(), at
>
>       https://doc.qt.io/qt-5/qlist.html#takeAt
>
> explicitly uses the word "removes":
>
>       Removes the item at index position i and returns it.
>
> So, yes, unless the intent is to modify the list, use QList::at().


___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev@xxxxxxxxxxxxx>
Archives:    https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request@xxxxxxxxxxxxx?subject=unsubscribe
___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev@xxxxxxxxxxxxx>
Archives:    https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
            mailto:wireshark-dev-request@xxxxxxxxxxxxx?subject=unsubscribe