search

MichaelChirico / r-bugs

A ⚠️read-only⚠️mirror of https://bugs.r-project.org/
20 stars 0 forks source link

[BUGZILLA #15438] Improvements to Quartz/Cocoa Device when Used from Terminal #5025

Open MichaelChirico opened 4 years ago

MichaelChirico commented 4 years ago

Created attachment 1479 [details] Changed files in grDevices

Here are a few changes in grDevices to improve the Cocoa/Quartz device appearance/functionality:

Getting a real application menu with About, Hide, Quit, etc. requires loading from an application package, which command line R obviously doesn't do. There are workarounds that call functions that were removed around the time of Mac OS X 10.4. (Google setAppleMenu for more details.) They can still be called in an unsupported fashion, but I don't think it's a good idea to be calling APIs that are now private.

This patch simply adds an empty menu item to the menu bar at index 0. (The File menu used to be at index 0, it is now index 1.) Under OS X 10.8, this results in correct menu names, although it is not guaranteed to work in the future. However, if the behavior changes it will hopefully not be worse than it was before this patch.

The R logo was taken from the R.app icon file. It is a 128x128 pixel PNG file located in a new directory, grDevices/inst/aqua. Makefile.in is set up to only install this directory if aqua is enabled. When a Cocoa/Quartz device is initially created, it finds this directory using R_HomeDir().

This is done using embedded AppleScripts in two class methods. Class (as opposed to instance) methods were used so the menu item is functional even if all Quartz device windows are closed. The menu item is only added if R was run from Terminal.app (as opposed to xterm, ssh, etc. Compiling/running the AppleScript that tests for this causes a small delay in opening the Quartz device. On my laptop this is only noticeable if you compare the old and new code side by side. If this very slight lag is unacceptable, the check could be removed, or potentially delayed until after the run loop starts (but that would increase complexity).

The attached archive has the files modified from the latest version of trunk.

The diff is below:

Index: grDevices/Makefile.in =================================================================== --- grDevices/Makefile.in (revision 63776) +++ grDevices/Makefile.in (working copy) @@ -17,7 +17,7 @@ R_EXE = $(top_builddir)/bin/R --vanilla --slave

RSRC = LC_COLLATE=C ls $(srcdir)/R/*.R $(srcdir)/R/$(R_OSTYPE)/*.R -INSTDIRS = afm enc icc +INSTDIRS = afm enc icc @BUILD_AQUA_TRUE@ aqua DEFPKGS = NULL

all: Makefile DESCRIPTION Index: grDevices/src/qdCocoa.m =================================================================== --- grDevices/src/qdCocoa.m (revision 63776) +++ grDevices/src/qdCocoa.m (working copy) @@ -30,6 +30,7 @@

include <R.h>

include

+#include

include <R_ext/QuartzDevice.h>

include <R_ext/eventloop.h>

@@ -113,14 +114,25 @@ / File menu is tricky - it may have a different name in different localizations. Hence we use a trick - the File menu should be first and have the shortcut for "Close Window" by convenience / BOOL hasFileMenu = NO; if (!soleMenu) { / in the case of a soleMenu we already know that we don't have it. Otherwise look for it. /

++ (BOOL)canShowTerminal +{ + if (!isatty(STDOUT_FILENO)) return NO; + char tty = ttyname(STDOUT_FILENO); +
+ NSString appleScriptSource = [[NSString alloc] initWithFormat: + @"tell application \"Terminal\"\n" + " if it is running then\n" + " repeat with theWindow in windows\n" + " repeat with theTab in (tabs of theWindow)\n" + " if tty of theTab is \"%s\" then\n" + " return true\n" + " end if\n" + " end repeat\n" + " end repeat\n" + " end if\n" + "end tell\n" + "return false", tty]; +
+ NSAppleScript appleScript = [[NSAppleScript alloc] initWithSource:appleScriptSource]; + NSDictionary errorInfo = nil; + NSAppleEventDescriptor appleEventDescriptor = [appleScript executeAndReturnError:&errorInfo]; +
+ [appleScriptSource release]; + [appleScript release]; +
+ if (errorInfo) return NO; + return [appleEventDescriptor booleanValue]; +} + ++ (void)showTerminal: (id) sender +{ + if (!isatty(STDOUT_FILENO)) return; + char tty = ttyname(STDOUT_FILENO); +
+ NSString appleScriptSource = [[NSString alloc] initWithFormat: + @"tell application \"Terminal\"\n" + " repeat with theWindow in windows\n" + " repeat with theTab in (tabs of theWindow)\n" + " if tty of theTab is \"%s\" then\n" + " if selected tab of theWindow is not theTab then\n" + " set selected tab of theWindow to theTab\n" + " end if\n" + " if index of theWindow is not 1 then\n" + " set frontmost of theWindow to true\n" + " end if\n" + " activate\n" + " return true\n" + " end if\n" + " end repeat\n" + " end repeat\n" + "end tell\n" + "return false", tty]; +
+ NSAppleScript appleScript = [[NSAppleScript alloc] initWithSource:appleScriptSource]; + NSDictionary *errorInfo = nil; + if ([appleScript compileAndReturnError:&errorInfo]) { + [appleScript executeAndReturnError:&errorInfo]; + } else { + NSLog(@"Error Compiling AppleScript: %@", errorInfo); + } +
+ [appleScriptSource release]; + [appleScript release]; +} + @end

pragma mark --- Cocoa event loop ---

METADATA

MichaelChirico commented 4 years ago

Created attachment 1480 [details] Changed/Added files in grDevices

Here is a revised set of new files that addresses several drawbacks with the previous approach.

Instead of programmatically creating the main menu, it is now loaded from a nib file. This is an officially supported way of creating a main menu and should be much more future proof. The nib file is stored in a bundle and could be localized if desired. The makefiles were updated to generate the nib file a xib file. If used the use of the nib obsoletes quite a bit of C code that could be removed. The largest blocks that are candidates for removal are marked with comments.

In addition, this makes the application menu more functional by including both an "About R" menu item and a working "Quit R" menu item. The "About R" menu item could be enhanced by including version/copyright information. I don't know how to get this information through C code. The "Quit R" currently switches to the Terminal (if being used) and displays a message about how to quit R. This could also be enhanced to actually quit and/or show the save workspace image prompt, but I don't know how to do that through C code.

The loading the mani menu also instantiates a new application delegate class, QuartzAppDelegate, which handles the quit and about actions. In addition, it handles the AppleScript code. Checking whether R is run from the terminal is now delayed from Cocoa initialization to subsequent run loop iterations, which removes the startup delay. The compiled show terminal AppleScript is cached so that subsequent uses are cached.

The diff against the current trunk is below:

Index: grDevices/Makefile.in =================================================================== --- grDevices/Makefile.in (revision 63830) +++ grDevices/Makefile.in (working copy) @@ -25,6 +25,8 @@ @$(MKINSTALLDIRS) $(top_builddir)/library/$(pkg) @$(MAKE) mkR1 mkdesc mkdemos instdirs @$(MAKE) mksrc +@BUILD_AQUA_TRUE@ @cp -R $(srcdir)/inst/aqua.bundle $(top_builddir)/library/$(pkg) +@BUILD_AQUA_TRUE@ @cp -R $(srcdir)/src/MainMenu.nib $(top_builddir)/library/$(pkg)/aqua.bundle/Contents/Resources @BYTE_COMPILE_PACKAGES_FALSE@ @$(MAKE) mklazy @BYTE_COMPILE_PACKAGES_TRUE@ @$(MAKE) mklazycomp @$(R_GZIPCMD) -9f $(top_builddir)/library/grDevices/afm/*.afm Index: grDevices/src/Makefile.in =================================================================== --- grDevices/src/Makefile.in (revision 63830) +++ grDevices/src/Makefile.in (working copy) @@ -35,6 +35,7 @@ all: Makefile Makedeps @$(MAKE) Makedeps @$(MAKE) shlib @BUILD_DEVCAIRO_TRUE@ cairodevice +@BUILD_AQUA_TRUE@ @$(MAKE) MainMenu.nib

Makefile: $(srcdir)/Makefile.in $(top_builddir)/config.status @cd $(top_builddir) && $(SHELL) ./config.status $(subdir)/$@ @@ -52,6 +53,9 @@ cairodevice: @(cd cairo && $(MAKE))

+MainMenu.nib: MainMenu.xib + ibtool --compile MainMenu.nib MainMenu.xib + include $(R_HOME)/etc${R_ARCH}/Makeconf include $(top_srcdir)/share/make/shlib.mk LTO = @LTO@ @@ -59,7 +63,7 @@ mostlyclean: clean clean: @-rm -rf .libs _libs

-@interface QuartzCocoaView : NSView +@interface QuartzCocoaView : NSView { QuartzCocoaDevice *ci; } @@ -51,4 +51,19 @@

@end

+@interface QuartzAppDelegate : NSObject { + + BOOL _isRunningInTerminal; + NSAppleScript _showTerminalAppleScript; +} + +@property(readonly) BOOL isRunningInTerminal; +@property(readonly) NSAppleScript showTerminalAppleScript; + +- (IBAction)showAboutPanel:(id)sender; +- (void)checkIsRunningInTerminal; +- (IBAction)showTerminal:(id)sender; + +@end +

endif

Index: grDevices/src/qdCocoa.m =================================================================== --- grDevices/src/qdCocoa.m (revision 63830) +++ grDevices/src/qdCocoa.m (working copy) @@ -30,6 +30,7 @@

include <R.h>

include

+#include

include <R_ext/QuartzDevice.h>

include <R_ext/eventloop.h>

@@ -94,6 +95,7 @@ NSColor *canvasColor = [view canvasColor]; [window setBackgroundColor:canvasColor ? canvasColor : [NSColor colorWithCalibratedRed:1.0 green:1.0 blue:1.0 alpha:0.5]]; [window setOpaque:NO]; + [window setReleasedWhenClosed:NO]; ci->window = window;

[window setDelegate: view];

@@ -110,6 +112,7 @@ if (soleMenu) [NSApp setMainMenu:[[NSMenu alloc] init]]; mainMenu = [NSApp mainMenu];

+ / This code is replaced by the MainMenu nib and can be removed / / File menu is tricky - it may have a different name in different localizations. Hence we use a trick - the File menu should be first and have the shortcut for "Close Window" by convenience / BOOL hasFileMenu = NO; if (!soleMenu) { / in the case of a soleMenu we already know that we don't have it. Otherwise look for it. / @@ -165,6 +168,7 @@ else / this should never be the case because we have added "File" menu, but just in case something goes wrong ... / [mainMenu addItem: menuItem]; } + / End code that can be removed /

     if ([mainMenu indexOfItemWithTitle:@"Quartz"] < 0) { /* Quartz menu - if it doesn't exist, add it */
         unichar leftArrow = NSLeftArrowFunctionKey, rightArrow = NSRightArrowFunctionKey;

@@ -186,6 +190,7 @@ [[NSApp mainMenu] addItem:menuItem]; } } + / This code is replaced by the MainMenu nib and can be removed / if (soleMenu) { / those should be standard if we have some menu / menu = [[NSMenu alloc] initWithTitle:@"Window"];

@@ -199,6 +204,7 @@ [NSApp setWindowsMenu:menu]; [menu release]; [menuItem release]; + / End code that can be removed / }
} @catch (NSException ex) { / on error release what we know about, issue a warning and return nil */ @@ -512,6 +518,8 @@

pragma mark --- Cocoa event loop ---

+static NSAutoreleasePool global_pool = 0; + / --- Cocoa event loop This EL is enabled upon the first use of Quartz or alternatively using the QuartzCocoa_SetupEventLoop function / @@ -548,7 +556,12 @@ untilDate:nil inMode:NSDefaultRunLoopMode dequeue:YES])) + { [NSApp sendEvent:event]; + [NSApp updateWindows]; + [global_pool release]; + global_pool = [[NSAutoreleasePool alloc] init]; + } el_pe_serial = el_serial; } } @@ -638,7 +651,7 @@ /----- R Quartz interface ------*/

static int cocoa_initialized = 0; -static NSAutoreleasePool global_pool = 0; +static NSBundle aquaBundle = nil;

static void initialize_cocoa() { / check embedding parameters to see if Rapp (or other Cocoa app) didn't do the work for us / @@ -652,7 +665,14 @@ return; }

METADATA

INCLUDED PATCH

github-actions[bot] commented 4 years ago

NA

METADATA

github-actions[bot] commented 4 years ago

NA

METADATA

github-actions[bot] commented 4 years ago

NA

METADATA

github-actions[bot] commented 4 years ago

NA

METADATA

github-actions[bot] commented 4 years ago

NA

METADATA