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

From: Roland Knall <rknall@xxxxxxxxx>
Date: Sat, 6 Aug 2022 14:01:51 +0200
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