[vlc-devel] [PATCH] macosx: First commit VLCStatusBarIcon code for macosx gui.

David Fuhrmann david.fuhrmann at gmail.com
Sun Jan 3 10:56:23 CET 2016


Hi,

Here are some mode detailed comments.

In general: Please set your Xcode to automatically trim all trailing whitespaces, they are quite some in your patch. Also take care of tabs vs. 4 spaces - most of the files use 4 spaces instead of a tab.

Other comments see inline.

> From 4fbf82bfabf1d299c9eb692813c437e841a13b1b Mon Sep 17 00:00:00 2001
> From: Goran Dokic <vlc at 8hz.com>
> Date: Sat, 2 Jan 2016 11:16:58 +0100
> Subject: [PATCH] macosx: First commit VLCStatusBarIcon code for macosx gui.
> 
>> ...
> diff --git a/Makefile.am b/Makefile.am
> index c853f52..3652aae 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -855,6 +855,13 @@ EXTRA_DIST += \
>  	extras/package/macosx/Resources/sidebar-icons_yosemite/ys-sidebar-playlist at 2x.png \
>  	extras/package/macosx/Resources/sidebar-icons_yosemite/ys-sidebar-podcast.png \
>  	extras/package/macosx/Resources/sidebar-icons_yosemite/ys-sidebar-podcast at 2x.png \
> +        extras/package/macosx/Resources/Media.xcassets/Contents.json \
> +        extras/package/macosx/Resources/Media.xcassets/MenuIcon.imageset/Contents.json \
> +        extras/package/macosx/Resources/Media.xcassets/MenuIcon.imageset/vlc-statusbar-icon-21x21.png \
> +        extras/package/macosx/Resources/Media.xcassets/MenuIcon.imageset/vlc-statusbar-icon-41x41.png \
> +	extras/package/macosx/Resources/icons/pause-19x19.png \
> +	extras/package/macosx/Resources/icons/play-19x19.png \
> +	extras/package/macosx/Resources/icons/stop-19x19.png \

Please check tabs vs. spacing. Here, tabs should be used.

>  	extras/package/macosx/Resources/vlc.scriptSuite \
>  	extras/package/macosx/Resources/vlc.scriptTerminology \
>  	extras/package/macosx/ub.sh \
> diff --git a/extras/package/macosx/Resources/Contents.json b/extras/package/macosx/Resources/Contents.json
> new file mode 100644
> index 0000000..da4a164
> --- /dev/null
> +++ b/extras/package/macosx/Resources/Contents.json
> @@ -0,0 +1,6 @@
> +{
> +  "info" : {
> +    "version" : 1,
> +    "author" : "xcode"
> +  }
> +}

No need to add that.

> \ No newline at end of file
> diff --git a/extras/package/macosx/Resources/English.lproj/VLCStatusBarIconMainMenu.xib b/extras/package/macosx/Resources/English.lproj/VLCStatusBarIconMainMenu.xib
> new file mode 100755
> index 0000000..0337ab0
> --- /dev/null
> +++ b/extras/package/macosx/Resources/English.lproj/VLCStatusBarIconMainMenu.xib
> @@ -0,0 +1,71 @@
> +<?xml version="1.0" encoding="UTF-8" standalone="no"?>
> +<document type="com.apple.InterfaceBuilder3.Cocoa.XIB" version="3.0" toolsVersion="9531" systemVersion="14F1509" targetRuntime="MacOSX.Cocoa" propertyAccessControl="none">
> +    <dependencies>
> +        <deployment identifier="macosx"/>
> +        <plugIn identifier="com.apple.InterfaceBuilder.CocoaPlugin" version="9531"/>
> +    </dependencies>
> +    <objects>
> +        <customObject id="-2" userLabel="File's Owner" customClass="NSApplication“>

You should set the class type of the files owner to VLCStatusBarIcon, see below.

> +            <connections>
> +                <outlet property="delegate" destination="533" id="495"/>
> +            </connections>
> +        </customObject>
> +        <customObject id="-1" userLabel="First Responder" customClass="FirstResponder"/>
> +        <customObject id="-3" userLabel="Application" customClass="NSObject"/>
> +        <customObject id="420" customClass="NSFontManager"/>
> +        <customObject id="533" customClass="VLCStatusBarIcon“>

No need to add a custom object with this class.

> +            <connections>
> +                <outlet property="VLCStatusBarIconMenu" destination="534" id="541"/>
> +            </connections>
> +        </customObject>
> +        <menu id="534" userLabel="VLCStatusBarIconMenu">
> +            <items>
> +                <menuItem title="Main WIndow" toolTip="Restore main window" id="08C-TA-yqu">
> +                    <modifierMask key="keyEquivalentModifierMask"/>
> +                    <connections>
> +                        <action selector="restoreMainWindow:" target="533" id="643"/>
> +                    </connections>
> +                </menuItem>
> +                <menuItem isSeparatorItem="YES" id="t2X-zn-Z4x"/>
> +                <menuItem title="Play" tag="74747" toolTip="Start/Pause playback" id="9kC-yJ-Gy5">
> +                    <modifierMask key="keyEquivalentModifierMask"/>
> +                    <connections>
> +                        <action selector="statusBarIconTogglePlayPause:" target="533" id="644"/>
> +                    </connections>
> +                </menuItem>
> +                <menuItem title="Stop" image="stop-19x19" toolTip="Stop playback" id="4Sh-MJ-bSf">
> +                    <connections>
> +                        <action selector="statusBarIconStop:" target="533" id="646"/>
> +                    </connections>
> +                </menuItem>
> +                <menuItem isSeparatorItem="YES" id="b5S-aL-b79"/>
> +                <menuItem title="Next" toolTip="Next track in playlist" id="GhQ-VQ-jtu">
> +                    <connections>
> +                        <action selector="statusBarIconNext:" target="533" id="647"/>
> +                    </connections>
> +                </menuItem>
> +                <menuItem title="Previous" toolTip="Previous track in playlist" id="iHu-rd-8KL">
> +                    <connections>
> +                        <action selector="statusBarIconPrevious:" target="533" id="648"/>
> +                    </connections>
> +                </menuItem>
> +                <menuItem isSeparatorItem="YES" id="Dn5-wA-WfF"/>
> +                <menuItem title="Random" id="lPi-EL-hoN">
> +                    <connections>
> +                        <action selector="statusBarIconToggleRandom:" target="533" id="649"/>
> +                    </connections>
> +                </menuItem>
> +                <menuItem isSeparatorItem="YES" id="p96-c8-lBM"/>
> +                <menuItem title="Quit" toolTip="Quit VLC" id="539">
> +                    <connections>
> +                        <action selector="quitAction:" target="533" id="542"/>
> +                    </connections>
> +                </menuItem>
> +            </items>
> +            <point key="canvasLocation" x="328.5" y="267.5"/>
> +        </menu>
> +    </objects>
> +    <resources>
> +        <image name="stop-19x19" width="19" height="19"/>
> +    </resources>
> +</document>
> diff --git a/extras/package/macosx/Resources/Media.xcassets/MenuIcon.imageset/Contents.json b/extras/package/macosx/Resources/Media.xcassets/MenuIcon.imageset/Contents.json
> new file mode 100644
> index 0000000..26b5694
> --- /dev/null
> +++ b/extras/package/macosx/Resources/Media.xcassets/MenuIcon.imageset/Contents.json
> @@ -0,0 +1,22 @@
> +{
> +  "images" : [
> +    {
> +      "idiom" : "universal",
> +      "filename" : "vlc-statusbar-icon-21x21.png",
> +      "scale" : "1x"
> +    },
> +    {
> +      "idiom" : "universal",
> +      "filename" : "vlc-statusbar-icon-41x41.png",
> +      "scale" : "2x"
> +    },
> +    {
> +      "idiom" : "universal",
> +      "scale" : "3x"
> +    }
> +  ],
> +  "info" : {
> +    "version" : 1,
> +    "author" : "xcode"
> +  }
> +}

There should be no need to add an image set for the icon, see below.

> \ No newline at end of file
> diff --git a/extras/package/macosx/Resources/Media.xcassets/MenuIcon.imageset/vlc-statusbar-icon-21x21.png b/extras/package/macosx/Resources/Media.xcassets/MenuIcon.imageset/vlc-statusbar-icon-21x21.png

Please adapt to naming schemes of existing icons. This probably should be names statusIcon.png, and the retina variant statusIcon at 2x.png. Then cocoa should pick the right one automatically. Then there should be no need to create the ImageSet file above.

> new file mode 100644
> index 0000000000000000000000000000000000000000..05a5d3e3cb85cad8719a77a04d782f4af52820e5
> GIT binary patch
> literal 572
> …..
> 
> literal 0
> HcmV?d00001
> 
> diff --git a/extras/package/macosx/Resources/Media.xcassets/MenuIcon.imageset/vlc-statusbar-icon-41x41.png b/extras/package/macosx/Resources/Media.xcassets/MenuIcon.imageset/vlc-statusbar-icon-41x41.png

41 pixel is not 2 x 21 pixel. Something must be wrong with the icon dimensions.

> new file mode 100644
> index 0000000000000000000000000000000000000000..64436d12e1c702f6d457e7e854276bcf16c1b729
> GIT binary patch
> literal 1296
> ...
> diff --git a/extras/package/macosx/Resources/icons/pause-19x19.png b/extras/package/macosx/Resources/icons/pause-19x19.png
> new file mode 100644

Same with other icons, regarding the naming scheme and retina versions.

> ...
>  			children = (
> diff --git a/modules/gui/macosx/Makefile.am b/modules/gui/macosx/Makefile.am
> index 691cf12..78282e5 100644
> --- a/modules/gui/macosx/Makefile.am
> +++ b/modules/gui/macosx/Makefile.am
> @@ -75,4 +75,5 @@ libmacosx_plugin_la_SOURCES = \
>  	VLCUIWidgets.h VLCUIWidgets.m \
>  	VLCScrollingClipView.h VLCScrollingClipView.m \
>  	VLCVoutWindowController.h VLCVoutWindowController.m \
> +	VLCStatusBarIcon.h VLCStatusBarIcon.m \

Please order alphabetically.

>  	Windows.h Windows.m
> diff --git a/modules/gui/macosx/VLCStatusBarIcon.h b/modules/gui/macosx/VLCStatusBarIcon.h
> new file mode 100644
> index 0000000..e11a894
> --- /dev/null
> +++ b/modules/gui/macosx/VLCStatusBarIcon.h
> @@ -0,0 +1,50 @@
> +//
> +//  VLCStatusBarIcon.h
> +//
> +// VLC macosx statusItem: Goran Dokic <vlc at 8hz com>
> +// v0.1
> +//
> +// some license
> +//
> +// THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS "AS IS" AND ANY
> +// EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
> +// WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
> +// DISCLAIMED. IN NO EVENT SHALL THE REGENTS AND CONTRIBUTORS BE LIABLE FOR ANY
> +// DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
> +// (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
> +// LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
> +// ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
> +// SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

Please use the GPL copyright header and style you find in other existing files of the mac interface.

> +
> +#import <Foundation/Foundation.h>

You actually need this?

> +#import <cocoa/cocoa.h>
> +
> +
> + at interface VLCStatusBarIcon : NSObject <NSMenuDelegate> {
> +    NSStatusItem *_statusItem;
> +    NSMenuItem *vlcStatusBarMenuItem;

Object variables should follow _camelCase style.

> +    IBOutlet NSMenu *VLCStatusBarIconMenu;

Wrong style. For outlets, you should create a property as done in most other files already.

> +}
> +
> + at property (getter=isPlaying) BOOL playing;
> + at property (getter=isRandom)  BOOL random;
> + at property NSTimer *dataRefreshUpdateTimer;
> +
> +// get data from VLC and update the little status menu
> +- (void) updateMenuFromVLCdata;
> +- (void) updateMenuItemRandom;
> +- (void) updateMenuItemPlayPause;
> +- (void) setDataUpdateTimer:(float) interval;

Those methods might need some factoring. Like use one method to update the menu on demand, another one to build the tooltip.

> +
> +
> +
> +- (IBAction) updateMenuItemContent:(id)sender;
> +- (IBAction) restoreMainWindow:(id)sender;
> +- (IBAction) statusBarIconTogglePlayPause:(id)sender;
> +- (IBAction) statusBarIconStop:(id)sender;
> +- (IBAction) statusBarIconNext:(id)sender;
> +- (IBAction) statusBarIconPrevious:(id)sender;
> +- (IBAction) statusBarIconToggleRandom:(id)sender;
> +
> + at end
> diff --git a/modules/gui/macosx/VLCStatusBarIcon.m b/modules/gui/macosx/VLCStatusBarIcon.m
> new file mode 100644
> index 0000000..b357b5f
> --- /dev/null
> +++ b/modules/gui/macosx/VLCStatusBarIcon.m
> @@ -0,0 +1,415 @@
> +//
> +//  VLCStatusBarIcon.m
> +//
> +// VLC macosx statusItem: Goran Dokic <vlc at 8hz com> 
> +// v0.1 - 12-2015
> +// 
> +// some license
> +//
> +// THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS "AS IS" AND ANY
> +// EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
> +// WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
> +// DISCLAIMED. IN NO EVENT SHALL THE REGENTS AND CONTRIBUTORS BE LIABLE FOR ANY
> +// DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
> +// (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
> +// LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
> +// ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
> +// SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

See above.

> +
> +#import "VLCStatusBarIcon.h"
> +
> +#import "MainMenu.h"
> +#import "intf.h"
> +
> +#import <vlc_common.h>
> +#import <vlc_playlist.h>
> +#import <vlc_input.h>
> +#import <CoreInteraction.h>
> +#import <StringUtility.h>
> +
> +// ==== Defines
> +
> +#define playPauseMenuItemTag 74747
> +#define dataUpdateTimerInterval 2.0
> +#define NSInitialToolTipDelayIn_ms 10
> +#define maxURLLengthInMenu 50
> +
> +#define debug 0

Should be removed for the final version. For debugging and testing, you can use VLCs logging mechanism (msg_Dbg(VLCIntf, …).

> +
> +// ==== Implementation ========================================================
> +
> + at implementation VLCStatusBarIcon {
> +    // need this in more methods
> +
> +    NSString *nameToDisplay;
> +    NSString *timeToDisplay;
> +    NSString *durationToDisplay;
> +    NSString *urlToDisplay;
> +    NSImage *menuImagePlay;
> +    NSImage *menuImagePause;
> +}

See style above.

> +
> +// ==== App init ==============================================================

You can use this to create some sensible sections:

#pragma mark -
#pragma mark Actions for menu items (e.g.)

> +
> ++(id)alloc 
> +{
> +    return [super alloc];
> +}
> +
> +- (id)init 
> +{
> +    return [super init];
> +}

No, just remove those if you do not have custom code.

> +
> +// fire it up
> +- (void) awakeFromNib
> +{
> +
> +    [super awakeFromNib];
> +
> +    _statusItem = [[NSStatusBar systemStatusBar] statusItemWithLength:NSVariableStatusItemLength];
> +    [_statusItem setHighlightMode:YES];
> +    [_statusItem setEnabled:YES];
> +    [_statusItem setToolTip:@"VLCStatusBarIcon“];

This is user visible text, so no. Just do not set any tooltip until you calculated the final string.

> +    [_statusItem setTarget:self];
> +
> +    // Attach pull-down menu
> +    [_statusItem setMenu:VLCStatusBarIconMenu];
> +
> +    // add the top menu item for dynamic data
> +    vlcStatusBarMenuItem = [[NSMenuItem alloc] initWithTitle:@"-- -- -- --"
> +                 action:@selector(updateMenuItemContent:) keyEquivalent:@"“];

No menus with dashes please.

> +
> +    [vlcStatusBarMenuItem setTarget:self];
> +    [VLCStatusBarIconMenu insertItem:vlcStatusBarMenuItem atIndex:0];
> +    
> +    // Set our selves up as delegate, to receive menuNeedsUpdate messages, so
> +    // we can update our menu as needed/before it's drawn
> +    [VLCStatusBarIconMenu setDelegate:self];
> +
> +    // set icon
> +    NSImage *menuIcon = [NSImage imageNamed:@"MenuIcon"];
> +    [_statusItem setImage:menuIcon];
> +	
> +    // make sure we invert the statusItem icon on dark/light mode 
> +     menuIcon.template = YES;
> +     _statusItem.image = menuIcon;

This is redundant and equivalent to [_statusItem setImage:menuIcon] above.

> +     _statusItem.highlightMode = YES;

Needed? And its deprecated.

> +
> +    // increase toolTip speed, improves status usability
> +    // Tweak delay above (#define)
> +
> +    [[NSUserDefaults standardUserDefaults] setObject: [NSNumber numberWithInt: NSInitialToolTipDelayIn_ms] forKey: @"NSInitialToolTipDelay“];

No need to save this timeout in prefs.

> +
> +    // init with 'Unknown' (this now means 'empty')
> +    urlToDisplay = @"Unknown";
> +
> +    // load the menu icons
> +    menuImagePlay = [NSImage imageNamed:@"play-19x19.png"];
> +    menuImagePause = [NSImage imageNamed:@"pause-19x19.png"];
> +    // .. Image for menu item 'Stop' is loaded from resources through xib
> +
> +    // I'd rather not use a timer and only update when mouse comes near
> +    // status icon in bar. But one can't tell without evil sourcery :(
> +    // Tweak update frequency above (#define)
> +
> +    [self setDataUpdateTimer:dataUpdateTimerInterval];
> +
> +} // end awakeFromNib
> +
> +// ==== Various functions =====================================================
> +
> +//--- 
> +// Menu delegate/callback for cocoa - called before menu is opened/displayed
> +// fire off menu item updates (URL, play/pause, random, etc)
> +
> +- (void)menuNeedsUpdate:(NSMenu *)menu 
> +{
> +    if (debug) NSLog(@"MenuNeedsUpdate callback/delegate called.\n");
> +
> +    [self updateMenuFromVLCdata];
> +}
> +
> +
> +
> +//---
> +// callback for tooltip update timer
> +// 
> +- (void) dataRefreshTimeHandler:(NSTimer *)timer 
> +{
> +    if (debug) NSLog(@"tooltipRefreshTimeHandler called.\n");
> +
> +    // get all interesting data from VLC
> +    [self gatherDataToDisplay];
> + 
> +    // update the toolTip with new data
> +    [self updateToolTipText];
> +
> +    // we can also update the dynamic menu item (item 0)
> +    [self updateDynamicMenuItemText];   
> +} 
> +
> +
> +//--- Call this before updateToolTipText and updateDynamicMenuItem
> +- (void) gatherDataToDisplay 
> +{
> +    // A nicer way than calling this through a timer would be to only
> +    // update the tooltip on mouse-over, but &c
> +
> +    mtime_t pos;
> +
> +    // get name of current item - clear first! used in if/then
> +    nameToDisplay = nil;
> +    nameToDisplay = [[VLCCoreInteraction sharedInstance] nameOfCurrentPlaylistItem];
> +
> +    // if we have no name, there is probably nothing playing. 
> +    // So no other useful data either.
> +
> +    if (nameToDisplay == nil) 
> +    {
> +        nameToDisplay = @"Name Unknown/Nothing playing";
> +        durationToDisplay = @"--:--";
> +        timeToDisplay = @"--:--";
> +        urlToDisplay = @"Unknown";
> +    } else {
> +        input_thread_t * p_input;
> +        p_input = pl_CurrentInput(VLCIntf);
> +        if (p_input) 
> +        {
> +            pos = var_GetInteger(p_input, "time") / CLOCK_FREQ;
> +            vlc_object_release(p_input); // must release or get segfault on quit
> +        }
> +
> +        // update our time counter
> +        timeToDisplay = [[VLCStringUtility sharedInstance] stringForTime:(long long) pos];
> +
> +        // get the duration (if it's there)
> +        int duration = [[VLCCoreInteraction sharedInstance] durationOfCurrentPlaylistItem];
> +        durationToDisplay = [[VLCStringUtility sharedInstance] stringForTime:(long long) duration];
> +
> +        // update the playing item's URL/Path
> +        urlToDisplay =  [[[VLCCoreInteraction sharedInstance] URLOfCurrentPlaylistItem] absoluteString];
> +    }
> +
> +    if (debug) NSLog(@"timeToDisplay: %@, nameToDisplay: %@, duration: %@, urlToDisplay: %@", timeToDisplay, nameToDisplay, durationToDisplay, urlToDisplay);
> +
> +} // end gatherDataToDisplay
> +
> +
> +//--- Call for periodic updates of tooltip text
> +- (void) updateToolTipText 
> +{
> +
> +    // craft the multiline string, for the tooltip
> +    NSString *toolTipText = [NSString stringWithFormat:@"VLC media player\nName: %@\nDuration: %@\nTime: %@\nURL/Path: %@", nameToDisplay, durationToDisplay, timeToDisplay, urlToDisplay];
> +
> +    [_statusItem setToolTip:toolTipText];
> +
> +} // end updateToolTipText
> +
> +
> +
> +//--- Call for periodic updates of dynamic menu item
> +- (void) updateDynamicMenuItemText 
> +{
> +    NSString *menuString = nil;
> +
> +    // create string for dynamic menu bit
> +    if ([urlToDisplay isEqualToString:@"Unknown"]) 
> +    {
> +         menuString =  @"-- -- -- --";
> +    } else {
> +         menuString = [NSString stringWithFormat:@"Copy URL: %@", urlToDisplay];
> +    }
> +
> +    // truncate the URL string before inserting in menu.. Too long is ugly.
> +    // .... fuuuu @30s does not work for NSStrings :(

Please only keep sensible comments.. :)

> +
> +    if ([menuString length] > maxURLLengthInMenu) 
> +    {
> +         // truncate URL text for menu, add '...' cue at the end
> +         [vlcStatusBarMenuItem setTitle:[NSString stringWithFormat:@"%@%@", [menuString substringToIndex:maxURLLengthInMenu],@"..."]];
> +    } else {
> +         // use full length, no '...' needed at the end
> +         [vlcStatusBarMenuItem setTitle:[NSString stringWithFormat:@"%@", menuString]];
> +
> +    }
> +}
> +
> +
> +//--- 
> +// set timer for tooltips updates and flee
> +//
> +- (void) setDataUpdateTimer:(float) interval 
> +{
> +    self.dataRefreshUpdateTimer = [NSTimer scheduledTimerWithTimeInterval:interval
> +    target:self
> +    selector:@selector(dataRefreshTimeHandler:)
> +    userInfo:nil
> +    repeats:YES];
> +}
> +
> +
> +
> +//--- 
> +- (void) updateMenuItemRandom 
> +{
> +
> +   // get current random status
> +    bool b_value;
> +    playlist_t *p_playlist = pl_Get(VLCIntf);
> +    b_value = var_GetBool(p_playlist, "random");
> +
> +    if (debug) NSLog(@"Random status from vlc is: %d. Setting menu item Random.", b_value);
> +
> +    // get menuitem 'Random'
> +    NSMenuItem* menuItemToChange = [VLCStatusBarIconMenu itemWithTitle:@"Random"];
> +    if (b_value)  
> +    {
> +        [menuItemToChange setState:NSOnState];
> +    } else {
> +        [menuItemToChange setState:NSOffState];
> +    }
> +} // end updateMenuItemRandom
> +
> +
> +//--- 
> +- (void) updateMenuItemTrackInfo 
> +{
> +    // get track info
> +    // update menu
> +} // end updateMenuItemTrackInfo 
> +
> +
> +
> +//--- 
> +- (void) updateMenuItemPlayPause 
> +{
> +
> +    bool vlcPlaying = false;
> +   
> +    // get the current title of the mainMenu 'play' menuitem, it will hold
> +    // it's status. Follow it in our status menu. 
> +
> +    NSString *titleString = [[[[VLCMain sharedInstance] mainMenu] play] title];

No, you are not allowed to access other IBOutlet properties of other classes.

> +
> +    // note: if vlc mainmenu item title says 'pause', this means vlc is playing
> +    if ([titleString isEqualToString:@"Pause"]) 
> +    {   
> +         vlcPlaying = true;
> +    } 
> +
> +    // Select our 'play/pause' menuItem and update it as necessary.
> +    // Tagged this menuitem in the xib, to be able to select it easily, 
> +    // despite the variable nature.. see #define above
> +
> +    NSMenuItem* menuItemToChange = [VLCStatusBarIconMenu itemWithTag:playPauseMenuItemTag];
> +
> +    if (vlcPlaying) 
> +    {
> +        [menuItemToChange setTitle:@"Pause"];
> +        [menuItemToChange setImage:menuImagePause];
> +    } else {
> +        [menuItemToChange setTitle:@"Play"];
> +        [menuItemToChange setImage:menuImagePlay];
> +    }
> +

As I told you on IRC: This should be triggered from setPlay / setPause in VLCMainMenu. Doing this based on the (potentially translated) title of an main menu item is too brittle.

> +} // end updateMenuItemPlayPause
> +
> +
> +
> +//--- Called when opening menu, to update status of various items
> +- (void) updateMenuFromVLCdata {
> +
> +    // menu item index0 (URL/Path) is updated in dataRefreshTimeHandler
> +
> +    // update play/pause status in status bar menu
> +    [self updateMenuItemPlayPause];
> +
> +    // update random status in status bar menu
> +    [self updateMenuItemRandom];
> +
> +} // end updateMenuFromVLCData
> +
> +
> +
> +
> +
> +// ==== Menu actions ==========================================================
> +
> +// action for dynamic menu index 0 
> +
> +- (IBAction) updateMenuItemContent:(id)sender 
> +{
> +    // We update this item through the tooltip/timer cycle. As all data 
> +    // is prepared there.
> +    // Here we offer to copy the url to the clipboard (useful imo ;-)
> +
> +    NSString *menuString;
> +
> +    if ([urlToDisplay isEqualToString:@"Unknown"]) 
> +    {
> + 	// Don't copy anything to clipboard, as the 'URL/path' is emtpy
> +    } else {
> +        // We have sane data, selecting menu item places URL/path in 
> +        // system copy/paste buffer
> +        [[NSPasteboard generalPasteboard] clearContents];
> +        [[NSPasteboard generalPasteboard] setString:urlToDisplay  forType:NSStringPboardType];        
> +    }
> +}
> +
> +// action for 'main window'
> +
> +- (IBAction)restoreMainWindow:(id)sender 
> +{
> +    // force our window to go to front (huzzah) and restore window
> +    [[VLCApplication sharedApplication] activateIgnoringOtherApps:YES];

You can also use NSapp activate….. Its a bit shorter. :-)

> +    [[[VLCMain sharedInstance] mainWindow] makeKeyAndOrderFront:sender];
> +}
> +
> +// action for 'toggle play/pause'
> +
> +- (IBAction)statusBarIconTogglePlayPause:(id)sender 
> +{
> +    [[VLCCoreInteraction sharedInstance] playOrPause];
> +}
> +
> +// action for 'stop'
> +
> +- (IBAction)statusBarIconStop:(id)sender 
> +{
> +    [[VLCCoreInteraction sharedInstance] stop];
> +}
> +
> +// action for 'Next track'
> +
> +- (IBAction)statusBarIconNext:(id)sender 
> +{
> +    [[VLCCoreInteraction sharedInstance] next];
> +}
> +
> +// action for 'previous track'
> +
> +- (IBAction)statusBarIconPrevious:(id)sender 
> +{
> +    [[VLCCoreInteraction sharedInstance] previous];
> +}
> +
> +// action to actually 'toggle VLC randomize playorder status' 
> +
> +- (IBAction)statusBarIconToggleRandom:(id)sender 
> +{
> +    [[VLCCoreInteraction sharedInstance] shuffle];
> +}
> +
> +// action voor 'quit'
> +
> +- (IBAction)quitAction:(id)sender 
> +{
> +    // clean timer, quit 
> +    [self.dataRefreshUpdateTimer invalidate];
> +    [[NSApplication sharedApplication] terminate:nil];
> +}
> +
> + at end
> diff --git a/modules/gui/macosx/intf.h b/modules/gui/macosx/intf.h
> index f3cc079..6e297fd 100644
> --- a/modules/gui/macosx/intf.h
> +++ b/modules/gui/macosx/intf.h
> @@ -74,6 +74,7 @@ static NSString * VLCInputChangedNotification = @"VLCInputChangedNotification";
>  @class VLCVideoEffects;
>  @class VLCConvertAndSave;
>  @class ExtensionsManager;
> + at class VLCStatusBarIcon;
>  
>  @interface VLCMain : NSObject <NSWindowDelegate, NSApplicationDelegate>
>  
> @@ -95,6 +96,7 @@ static NSString * VLCInputChangedNotification = @"VLCInputChangedNotification";
>  - (ResumeDialogController *)resumeDialog;
>  - (VLCInputManager *)inputManager;
>  - (ExtensionsManager *)extensionsManager;
> +- (VLCStatusBarIcon *)statusBarIcon;
>  
>  - (VLCDebugMessageVisualizer *)debugMsgPanel;
>  
> diff --git a/modules/gui/macosx/intf.m b/modules/gui/macosx/intf.m
> index 63d3e43..9705c75 100644
> --- a/modules/gui/macosx/intf.m
> +++ b/modules/gui/macosx/intf.m
> @@ -65,6 +65,8 @@
>  #import "AudioEffects.h"
>  #import "intf-prefs.h"
>  
> +#import "VLCStatusBarIcon.h"
> +
>  #ifdef HAVE_SPARKLE
>  #import <Sparkle/Sparkle.h>                 /* we're the update delegate */
>  #endif
> @@ -94,7 +96,10 @@ int OpenIntf (vlc_object_t *p_this)
>          [VLCMain sharedInstance];
>  
>          [NSBundle loadNibNamed:@"MainMenu" owner:[[VLCMain sharedInstance] mainMenu]];
> +        [NSBundle loadNibNamed:@"VLCStatusBarIconMainMenu" owner:[[VLCMain sharedInstance] statusBarIcon]];

This integration does not work. You need to instantiate your object in code, which is missing here. Then, as stated above already, set your class name for the Files Owner object, and attach all your outlets / actions to files owner. The extra instance of VLCStatusBarIcon (created in the interface builder) can be removed, then.

Generally, maybe its better to hook your extra menu within VLCMainMenu instead. Because your menu is similar to main menu and the dock menu, which are both handled in this class.

> +
>          [[[VLCMain sharedInstance] mainWindow] makeKeyAndOrderFront:nil];
> +	

Trailing whitespaces, no need to change that.

>  
>          msg_Dbg(p_intf, "Finished loading macosx interface");
>          return VLC_SUCCESS;
> @@ -165,6 +170,7 @@ static int ShowController(vlc_object_t *p_this, const char *psz_variable,
>      ResumeDialogController *_resume_dialog;
>      VLCInputManager *_input_manager;
>      VLCPlaylist *_playlist;
> +    VLCStatusBarIcon *_statusBarIcon;
>      VLCDebugMessageVisualizer *_messagePanelController;
>      VLCTrackSynchronization *_trackSyncPanel;
>      VLCAudioEffects *_audioEffectsPanel;
> @@ -221,7 +227,7 @@ static VLCMain *sharedInstance = nil;
>          _voutController = [[VLCVoutWindowController alloc] init];
>          _playlist = [[VLCPlaylist alloc] init];
>  
> -        _mainWindowController = [[NSWindowController alloc] initWithWindowNibName:@"MainWindow"];
> +	_mainWindowController = [[NSWindowController alloc] initWithWindowNibName:@"MainWindow“];

Please do not change from 4 spaces to tabs.

>  
>          var_Create(p_intf, "intf-change", VLC_VAR_BOOL);
>  
> @@ -499,6 +505,11 @@ static VLCMain *sharedInstance = nil;
>      return _mainmenu;
>  }
>  
> +- (VLCStatusBarIcon *)statusBarIcon
> +{
> +    return _statusBarIcon;
> +}
> +
>  - (VLCMainWindow *)mainWindow
>  {
>      return (VLCMainWindow *)[_mainWindowController window];
> -- 
> 2.5.4 (Apple Git-61)
> 

Best regards,
David


More information about the vlc-devel mailing list