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

[Victorien Le Couviour--Tuffet]

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.

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.