[vlc-devel] [PATCH 04/23] macosx: refactor VLCVideoEffectsWindowController::resetValues

[Marvin Scholz]

On 29 Jun 2017, at 15:53, Victorien Le Couviour--Tuffet wrote:

> On Thu, Jun 29, 2017 at 03:18:53PM +0200, Marvin Scholz wrote:
>> On 29 Jun 2017, at 14:24, Victorien Le Couviour--Tuffet wrote:
>>>      [_transformCheckbox setTitle:_NS("Transform")];
>>>      [_transformPopup removeAllItems];
>>>      [_transformPopup addItemWithTitle: _NS("Rotate by 90 
>>> degrees")];
>>> +    [[_transformPopup lastItem] setAttributedTitle:
>>> [[NSAttributedString alloc] initWithString: @"90"]];
>>>      [[_transformPopup lastItem] setTag: 90];
>>>      [_transformPopup addItemWithTitle: _NS("Rotate by 180 
>>> degrees")];
>>> +    [[_transformPopup lastItem] setAttributedTitle:
>>> [[NSAttributedString alloc] initWithString: @"180"]];
>>>      [[_transformPopup lastItem] setTag: 180];
>>>      [_transformPopup addItemWithTitle: _NS("Rotate by 270 
>>> degrees")];
>>> +    [[_transformPopup lastItem] setAttributedTitle:
>>> [[NSAttributedString alloc] initWithString: @"270"]];
>>>      [[_transformPopup lastItem] setTag: 270];
>>>      [_transformPopup addItemWithTitle: _NS("Flip horizontally")];
>>> +    [[_transformPopup lastItem] setAttributedTitle:
>>> [[NSAttributedString alloc] initWithString: @"hflip"]];
>>>      [[_transformPopup lastItem] setTag: 1];
>>>      [_transformPopup addItemWithTitle: _NS("Flip vertically")];
>>> +    [[_transformPopup lastItem] setAttributedTitle:
>>> [[NSAttributedString alloc] initWithString: @"vflip"]];
>>>      [[_transformPopup lastItem] setTag: 2];
>>>      [_zoomCheckbox setTitle:_NS("Magnification/Zoom")];
>>>      [_puzzleCheckbox setTitle:_NS("Puzzle game")];
>>>      [_puzzleRowsLabel setStringValue:_NS("Rows")];
>>> +    [_puzzleRowsTextField setAttributedStringValue:
>>> [[NSAttributedString alloc] initWithString: @"isSpinBox"]];
>>>      [_puzzleColumnsLabel setStringValue:_NS("Columns")];
>>> +    [_puzzleColumnsTextField setAttributedStringValue:
>>> [[NSAttributedString alloc] initWithString: @"isSpinBox"]];
>>>      [_cloneCheckbox setTitle:_NS("Clone")];
>>>      [_cloneNumberLabel setStringValue:_NS("Number of clones")];
>>> +    [_cloneNumberTextField setAttributedStringValue:
>>> [[NSAttributedString alloc] initWithString: @"isSpinBox"]];
>>>      [_wallCheckbox setTitle:_NS("Wall")];
>>>      [_wallNumbersOfRowsLabel setStringValue:_NS("Rows")];
>>> +    [_wallNumbersOfRowsTextField setAttributedStringValue:
>>> [[NSAttributedString alloc] initWithString: @"isSpinBox"]];
>>>      [_wallNumberOfColumnsLabel setStringValue:_NS("Columns")];
>>> +    [_wallNumberOfColumnsTextField setAttributedStringValue:
>>> [[NSAttributedString alloc] initWithString: @"isSpinBox"]];
>>
>> Why do you set the attributedStrings here? I do not fully understand 
>> whats
>> the goal of doing so.
>
> This is just a little hack to help refactoring.
>
> The isSpinBox is for this piece of code:
> +        else if ([widget isKindOfClass: [NSTextField class]])
> +        {
> +            if ([[[widget attributedStringValue] string] compare: 
> @"isSpinBox"] == NSOrderedSame)
> +                [widget setStringValue: [[NSString alloc] 
> initWithFormat: @"%lli", val.i_int]];
> +            else
> +                [widget setStringValue: [[NSString alloc] 
> initWithFormat: @"%06" PRIX64, val.i_int]];
> +        }
>
> As there is no NSSpinBox like QSpinBox for Qt we cannot know if the 
> text fields
> have a stepper beside them. So in order to display either in base 10 
> (for
> spinbox widgets) or in base 16 for text field ones, I have to do this.

But you can't just set an attributed string value, this is what the text 
cell will display
and the same as the string value.

The correct way to do this is: set the intValue, floatValue or 
doubleValue
and use a text formatter, I think.

>
> The attributed strings in the popup items is for this piece of code:
> +        if ([widget isKindOfClass: [NSPopUpButton class]])
> +        {
> +            for (NSMenuItem *item in [widget itemArray])
> +                if ([item attributedTitle] &&
> +                    !strcmp([[[item attributedTitle] string] 
> UTF8String], val.psz_string))
> +                {
> +                    [widget selectItemWithTitle: [item title]];
> +                    break;
> +                }
> +        }
>
> This allows to select the active popup item according to the value of 
> the
> variable instead of hardcoding it as it was before.
>
> Without this I don't think of any generic way that would allow us to 
> refactor
> this function, which cause will the commit
> 'macosx: duplicate video filters options in the playlist' to add a 
> very big
> amount of redundant code.
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel