Re: [PATCH v2] ui/cocoa: Use the standard about panel

[Akihiko Odaki]

On Sun, Feb 27, 2022 at 2:18 AM BALATON Zoltan <balaton@eik.bme.hu> wrote:
>
> On Sat, 26 Feb 2022, Akihiko Odaki wrote:
> > This provides standard look and feel for the about panel and reduces
> > code.
> >
> > Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
> > ---
> > ui/cocoa.m | 112 +++++++++++------------------------------------------
> > 1 file changed, 23 insertions(+), 89 deletions(-)
> >
> > diff --git a/ui/cocoa.m b/ui/cocoa.m
> > index a8f1cdaf926..9c27b9f5aa7 100644
> > --- a/ui/cocoa.m
> > +++ b/ui/cocoa.m
> > @@ -83,7 +83,7 @@ static void cocoa_switch(DisplayChangeListener *dcl,
> >
> > static void cocoa_refresh(DisplayChangeListener *dcl);
> >
> > -static NSWindow *normalWindow, *about_window;
> > +static NSWindow *normalWindow;
> > static const DisplayChangeListenerOps dcl_ops = {
> >     .dpy_name          = "cocoa",
> >     .dpy_gfx_update = cocoa_update,
> > @@ -1120,7 +1120,6 @@ - (void)changeDeviceMedia:(id)sender;
> > - (BOOL)verifyQuit;
> > - (void)openDocumentation:(NSString *)filename;
> > - (IBAction) do_about_menu_item: (id) sender;
> > -- (void)make_about_window;
> > - (void)adjustSpeed:(id)sender;
> > @end
> >
> > @@ -1166,8 +1165,6 @@ - (id) init
> >         [pauseLabel setFont: [NSFont fontWithName: @"Helvetica" size: 90]];
> >         [pauseLabel setTextColor: [NSColor blackColor]];
> >         [pauseLabel sizeToFit];
> > -
> > -        [self make_about_window];
> >     }
> >     return self;
> > }
> > @@ -1451,92 +1448,29 @@ - (BOOL)verifyQuit
> > /* The action method for the About menu item */
> > - (IBAction) do_about_menu_item: (id) sender
> > {
> > -    [about_window makeKeyAndOrderFront: nil];
> > -}
> > -
> > -/* Create and display the about dialog */
> > -- (void)make_about_window
> > -{
> > -    /* Make the window */
> > -    int x = 0, y = 0, about_width = 400, about_height = 200;
> > -    NSRect window_rect = NSMakeRect(x, y, about_width, about_height);
> > -    about_window = [[NSWindow alloc] initWithContentRect:window_rect
> > -                    styleMask:NSWindowStyleMaskTitled | 
> > NSWindowStyleMaskClosable |
> > -                    NSWindowStyleMaskMiniaturizable
> > -                    backing:NSBackingStoreBuffered
> > -                    defer:NO];
> > -    [about_window setTitle: @"About"];
> > -    [about_window setReleasedWhenClosed: NO];
> > -    [about_window center];
> > -    NSView *superView = [about_window contentView];
> > -
> > -    /* Create the dimensions of the picture */
> > -    int picture_width = 80, picture_height = 80;
> > -    x = (about_width - picture_width)/2;
> > -    y = about_height - picture_height - 10;
> > -    NSRect picture_rect = NSMakeRect(x, y, picture_width, picture_height);
> > -
> > -    /* Make the picture of QEMU */
> > -    NSImageView *picture_view = [[NSImageView alloc] initWithFrame:
> > -                                                     picture_rect];
> > -    char *qemu_image_path_c = get_relocated_path(CONFIG_QEMU_ICONDIR 
> > "/hicolor/512x512/apps/qemu.png");
> > -    NSString *qemu_image_path = [NSString 
> > stringWithUTF8String:qemu_image_path_c];
> > -    g_free(qemu_image_path_c);
> > -    NSImage *qemu_image = [[NSImage alloc] 
> > initWithContentsOfFile:qemu_image_path];
> > -    [picture_view setImage: qemu_image];
> > -    [picture_view setImageScaling: NSImageScaleProportionallyUpOrDown];
> > -    [superView addSubview: picture_view];
> > -
> > -    /* Make the name label */
> > -    NSBundle *bundle = [NSBundle mainBundle];
> > -    if (bundle) {
> > -        x = 0;
> > -        y = y - 25;
> > -        int name_width = about_width, name_height = 20;
> > -        NSRect name_rect = NSMakeRect(x, y, name_width, name_height);
> > -        NSTextField *name_label = [[NSTextField alloc] initWithFrame: 
> > name_rect];
> > -        [name_label setEditable: NO];
> > -        [name_label setBezeled: NO];
> > -        [name_label setDrawsBackground: NO];
> > -        [name_label setAlignment: NSTextAlignmentCenter];
> > -        NSString *qemu_name = [[bundle executablePath] lastPathComponent];
> > -        [name_label setStringValue: qemu_name];
> > -        [superView addSubview: name_label];
> > +    NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
>
> Extra space between * and pool?

I'll remove it with the next version.

>
> > +    char *icon_path_c = get_relocated_path(CONFIG_QEMU_ICONDIR 
> > "/hicolor/512x512/apps/qemu.png");
> > +    NSString *icon_path = [NSString stringWithUTF8String:icon_path_c];
> > +    g_free(icon_path_c);
> > +    NSImage *icon = [[NSImage alloc] initWithContentsOfFile:icon_path];
> > +    NSString *version = @"QEMU emulator version " QEMU_FULL_VERSION;
> > +    NSString *copyright = @QEMU_COPYRIGHT;
> > +    NSDictionary *options;
> > +    if (icon) {
> > +        options = @{
> > +            NSAboutPanelOptionApplicationIcon : icon,
> > +            NSAboutPanelOptionApplicationVersion : version,
> > +            @"Copyright" : copyright,
> > +        };
> > +        [icon release];
> > +    } else {
> > +        options = @{
> > +            NSAboutPanelOptionApplicationVersion : version,
> > +            @"Copyright" : copyright,
> > +        };
>
> It may be less redundant to init the oprions dict only with version and
> copyright first then have an if adding the icon key if exists but for that
> an NSMutableDictionary may be needed so not sure about that. It might
> still have less duplication that way.

As you suspected, it would need NSMutableDictionary and conversion to
NSDictionary from it, and does not reduce the complexity in total.

Regards,
Akihiko Odaki

>
> Regards,
> BALATON Zoltan
>
> >     }
> > -
> > -    /* Set the version label's attributes */
> > -    x = 0;
> > -    y = 50;
> > -    int version_width = about_width, version_height = 20;
> > -    NSRect version_rect = NSMakeRect(x, y, version_width, version_height);
> > -    NSTextField *version_label = [[NSTextField alloc] initWithFrame:
> > -                                                      version_rect];
> > -    [version_label setEditable: NO];
> > -    [version_label setBezeled: NO];
> > -    [version_label setAlignment: NSTextAlignmentCenter];
> > -    [version_label setDrawsBackground: NO];
> > -
> > -    /* Create the version string*/
> > -    NSString *version_string;
> > -    version_string = [[NSString alloc] initWithFormat:
> > -    @"QEMU emulator version %s", QEMU_FULL_VERSION];
> > -    [version_label setStringValue: version_string];
> > -    [superView addSubview: version_label];
> > -
> > -    /* Make copyright label */
> > -    x = 0;
> > -    y = 35;
> > -    int copyright_width = about_width, copyright_height = 20;
> > -    NSRect copyright_rect = NSMakeRect(x, y, copyright_width, 
> > copyright_height);
> > -    NSTextField *copyright_label = [[NSTextField alloc] initWithFrame:
> > -                                                        copyright_rect];
> > -    [copyright_label setEditable: NO];
> > -    [copyright_label setBezeled: NO];
> > -    [copyright_label setDrawsBackground: NO];
> > -    [copyright_label setAlignment: NSTextAlignmentCenter];
> > -    [copyright_label setStringValue: [NSString stringWithFormat: @"%s",
> > -                                     QEMU_COPYRIGHT]];
> > -    [superView addSubview: copyright_label];
> > +    [NSApp orderFrontStandardAboutPanelWithOptions:options];
> > +    [pool release];
> > }
> >
> > /* Used by the Speed menu items */
> >