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

[Victorien Le Couviour--Tuffet]

On Thu, Jun 29, 2017 at 05:10:58PM +0200, Marvin Scholz wrote:
> On 29 Jun 2017, at 16:46, Victorien Le Couviour--Tuffet wrote:
> > On Thu, Jun 29, 2017 at 04:13:09PM +0200, Marvin Scholz wrote:
> > > 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.
> > > 
> > 
> > I'm not sure if I get you right, so do you mean something like this?
> > 
> > NSNumberFormatter *fmt = [[NSNumberFormatter alloc] init];
> > [fmt setFormat: @"%lli"];
> > [[_wallNumbersOfRowsTextField cell] setFormatter: fmt];
> > 
> > ... and then in setWidgetValue
> > 
> > else if ([widget isKindOfClass: [NSTextField class]])
> >     [widget setIntValue: val.i_int];
> > 
> > But cell is deprecated and I don't find any equivalent in the
> > documentation, so
> > if you know what is the new way to retrieve it so I can register the
> > formatter
> > on it, could you share it please?
> 
> Yes, that's what I meant.
> 
> You can just use [_textField setFormatter:<#(__kindof NSFormatter *
> _Nullable)#>] directly
> without accessing the cell.

Great, thanks! I don't have access to the Mac rigth now and not until tomorrow
evening. So I won't be able to send a patch with this correction before.