Text size setting (#620)

* Add font size user defaults key

Users should be able to control text size. The value in this user defaults key is the starting point for allowing any part of the GitUp UI to be scalable.

One limitation of storing the font size in the standard user defaults is that this is global state so an app using GitUpKit that wants to show two diff views at the same time would not be able to make them use different font sizes. I don’t think this use case is particularly important.

As a framework, GitUpKit should not register default values in user defaults, and it can’t guarantee that the app using GitUpKit registers a sensible default for this key. Therefore the font size should always be read from user defaults wrapped in a `GIFontSize()` function call, which ensures the value is greater than zero

The `GIFontSize` function was put in `GIAppKit.h/m` because that’s where other GitUpKit user defaults keys are defined. I didn’t think this fits in in `GIFunctions` because it’s only available in the Mac version of GitUpKit. Putting it in `GIFunctions` wrapped in `#if __GI_HAS_APPKIT__` would work too but makes less sense to me.

I thought it would be messy to do the `#ifdef __cplusplus` like in `GIFunctions` for this one function, so I used `FOUNDATION_EXPORT`. (My preferred way to do this would be to drop the `#ifdef __cplusplus` from `GIFunctions` and define a `GIEXPORT` used for every public function.)

Naming of the constant: I mirrored `GICommitMessageViewUserDefaultKey_ShowMargins` but removed the `CommitMessageView` part because this one is more general.

* Update MainMenu.xib

Modifications made automatically by Xcode 11.3 when opening first making a change to the file. There is nothing meaningful. This has been separated from the real change that is coming in the next commit.

* Make commit message view scale with font size

Must call `setNeedsDisplay:` otherwise the 50 and 72 character markers are sometimes not updated (such as when there is selected text).

The font size is 10% larger than the font size in user defaults, to match before where the font size here was 11 points compared to 10 in the diff view.

* Add slider in Preferences to change font size

Letting the user specify a scale but the application specify the typefaces and relative sizes seems like the best trade-off between allowing the user to see the app comfortably without asking them to be the designer. Also different views have use different fonts and relative sizes. Therefore, the user does not set an explicit typeface name or numerical size and instead a slider without value labels has been added to General Preferences to set the scale. This (deliberately) results in the exact size used hard to find.

I decided to store the most obvious thing in user defaults: the font size used in the diff view in points. This simplicity comes at the cost of making the slider in Preferences more complicated as a linear slider would not be very good: the difference between 10 and 11 points is much more important than the difference between 18 and 19 points. A value transformer is used to map between the discrete slider value and a non-linear array of font sizes that match the preset sizes in the Mac system font picker.

* Make diff view scale with font size

The font size matches the font size from user defaults directly.

The global variables associated with the diff view that have become properties on instances that update in `updateMetricsFromCurrentFontSize` when the user defaults change.

Constants have been replaced by functions.

The table view should always be reloaded when the font size changes to ensure the content size is correct.

The threshold for switching from unified to split diff is now specified as a multiple of the font size. Similar scaling is used for drawing the column separators for line numbers and positioning text. This ensures the layout is the same as GitUp was designed except for being scaled.

Author: douglashill

GitUp/Application/AppDelegate.m

@@ -93,6 +93,7 @@ + (void)initialize {
     GICommitMessageViewUserDefaultKey_ShowInvisibleCharacters : @(YES),
     GICommitMessageViewUserDefaultKey_ShowMargins : @(YES),
     GICommitMessageViewUserDefaultKey_EnableSpellChecking : @(YES),
+    GIUserDefaultKey_FontSize : @(GIDefaultFontSize),
     kUserDefaultsKey_ReleaseChannel : kReleaseChannel_Stable,
     kUserDefaultsKey_CheckInterval : @(15 * 60),
     kUserDefaultsKey_FirstLaunch : @(YES),

GitUp/Application/Base.lproj/MainMenu.xib

@@ -0,0 +1,21 @@
+//  Copyright (C) 2015-2019 Pierre-Olivier Latour <[email protected]>
+//
+//  This program is free software: you can redistribute it and/or modify
+//  it under the terms of the GNU General Public License as published by
+//  the Free Software Foundation, either version 3 of the License, or
+//  (at your option) any later version.
+//
+//  This program is distributed in the hope that it will be useful,
+//  but WITHOUT ANY WARRANTY; without even the implied warranty of
+//  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+//  GNU General Public License for more details.
+//
+//  You should have received a copy of the GNU General Public License
+//  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+#import <Foundation/Foundation.h>
+
+/// Transforms between the linear slider in Preferences to a non-linear list of font sizes matching the system font picker.
+@interface FontSizeTransformer : NSValueTransformer
+
+@end

GitUp/Application/FontSizeTransformer.h

@@ -0,0 +1,55 @@
+//  Copyright (C) 2015-2019 Pierre-Olivier Latour <[email protected]>
+//
+//  This program is free software: you can redistribute it and/or modify
+//  it under the terms of the GNU General Public License as published by
+//  the Free Software Foundation, either version 3 of the License, or
+//  (at your option) any later version.
+//
+//  This program is distributed in the hope that it will be useful,
+//  but WITHOUT ANY WARRANTY; without even the implied warranty of
+//  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+//  GNU General Public License for more details.
+//
+//  You should have received a copy of the GNU General Public License
+//  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+#import "FontSizeTransformer.h"
+#import <GitUpKit/GIAppKit.h>
+
+static NSArray* sizes;
+
+@implementation FontSizeTransformer
+
++ (void)initialize {
+  // Match the system font picker
+  sizes = @[ @9, @10, @11, @12, @13, @14, @18, @24 ];
+}
+
++ (Class)transformedValueClass {
+  return [NSNumber class];
+}
+
++ (BOOL)allowsReverseTransformation {
+  return YES;
+}
+
+- (id)transformedValue:(id)fontSizeValue {
+  NSUInteger idx = [sizes indexOfObject:fontSizeValue];
+  if (idx == NSNotFound) {
+    // If the user default is set externally, fallback to the default index.
+    return @1;
+  }
+
+  return @(idx);
+}
+
+- (id)reverseTransformedValue:(id)indexValue {
+  NSUInteger idx = [indexValue unsignedIntegerValue];
+  if (idx >= sizes.count) {
+    return @(GIDefaultFontSize);
+  }
+
+  return sizes[idx];
+}
+
+@end

GitUp/Application/FontSizeTransformer.m

@@ -19,6 +19,7 @@
 		0AE7F5F12312C1B000B06050 /* InfoPlist.strings in Resources */ = {isa = PBXBuildFile; fileRef = 0AE7F5ED2312C1B000B06050 /* InfoPlist.strings */; };
 		165C32B61B95739700D2F894 /* QuartzCore.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 165C32B51B95739700D2F894 /* QuartzCore.framework */; };
 		31CD50E3203E2E2800360B3A /* ToolbarItemWrapperView.m in Sources */ = {isa = PBXBuildFile; fileRef = 31CD50E2203E2E2800360B3A /* ToolbarItemWrapperView.m */; };
+		A53C6D0C1E61A9CF0070387E /* FontSizeTransformer.m in Sources */ = {isa = PBXBuildFile; fileRef = A53C6D0B1E61A9CF0070387E /* FontSizeTransformer.m */; };
 		E212A6DE1B92100E00F62B18 /* libiconv.dylib in Frameworks */ = {isa = PBXBuildFile; fileRef = E212A6DD1B92100E00F62B18 /* libiconv.dylib */; };
 		E21739F71A5080DD00EC6777 /* DocumentController.m in Sources */ = {isa = PBXBuildFile; fileRef = E21739F61A5080DD00EC6777 /* DocumentController.m */; };
 		E21DCAEB1B253847006424E8 /* main.m in Sources */ = {isa = PBXBuildFile; fileRef = E21DCAEA1B253847006424E8 /* main.m */; };
@@ -134,6 +135,8 @@
 		165C32B51B95739700D2F894 /* QuartzCore.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = QuartzCore.framework; path = System/Library/Frameworks/QuartzCore.framework; sourceTree = SDKROOT; };
 		31CD50E1203E2E2800360B3A /* ToolbarItemWrapperView.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = ToolbarItemWrapperView.h; sourceTree = "<group>"; };
 		31CD50E2203E2E2800360B3A /* ToolbarItemWrapperView.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = ToolbarItemWrapperView.m; sourceTree = "<group>"; };
+		A53C6D0A1E61A9CF0070387E /* FontSizeTransformer.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = FontSizeTransformer.h; sourceTree = "<group>"; };
+		A53C6D0B1E61A9CF0070387E /* FontSizeTransformer.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = FontSizeTransformer.m; sourceTree = "<group>"; };
 		E212A6DD1B92100E00F62B18 /* libiconv.dylib */ = {isa = PBXFileReference; lastKnownFileType = "compiled.mach-o.dylib"; name = libiconv.dylib; path = usr/lib/libiconv.dylib; sourceTree = SDKROOT; };
 		E21739F51A5080DD00EC6777 /* DocumentController.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = DocumentController.h; sourceTree = "<group>"; };
 		E21739F61A5080DD00EC6777 /* DocumentController.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = DocumentController.m; sourceTree = "<group>"; };
@@ -295,6 +298,8 @@
 				0AE7F5ED2312C1B000B06050 /* InfoPlist.strings */,
 				E2C338B319F8562F00063D95 /* main.m */,
 				E2C338BD19F8562F00063D95 /* MainMenu.xib */,
+				A53C6D0A1E61A9CF0070387E /* FontSizeTransformer.h */,
+				A53C6D0B1E61A9CF0070387E /* FontSizeTransformer.m */,
 				E2C5672B1A6D98BC00ECFE07 /* WindowController.h */,
 				E2C5672C1A6D98BC00ECFE07 /* WindowController.m */,
 				31CD50E1203E2E2800360B3A /* ToolbarItemWrapperView.h */,
@@ -471,6 +476,7 @@
 				E2E3C9A01D771E7600AA9A62 /* GARawTracker.m in Sources */,
 				E2C338B419F8562F00063D95 /* main.m in Sources */,
 				E2C5672D1A6D98BC00ECFE07 /* WindowController.m in Sources */,
+				A53C6D0C1E61A9CF0070387E /* FontSizeTransformer.m in Sources */,
 				E2C338B219F8562F00063D95 /* AppDelegate.m in Sources */,
 				0A58CD77237B4F4B00C2BDD0 /* CloneWindowController.m in Sources */,
 				E2C338B719F8562F00063D95 /* Document.m in Sources */,

GitUp/GitUp.xcodeproj/project.pbxproj

@@ -24,7 +24,8 @@
 #import "GCRepository+Index.h"
 #import "XLFacilityMacros.h"
 
-#define kMinSplitDiffViewWidth 1000
+// Units ems: a multiple of the font point size, so the width threshold is 100 * 10 = 1000 for a 10 point font.
+#define kMinSplitDiffViewWidthEms 100
 
 #define kContextualMenuOffsetX 0
 #define kContextualMenuOffsetY -6
@@ -189,13 +190,15 @@ + (void)initialize {
 - (instancetype)initWithRepository:(GCLiveRepository*)repository {
   if ((self = [super initWithRepository:repository])) {
     [[NSUserDefaults standardUserDefaults] addObserver:self forKeyPath:GIDiffContentsViewControllerUserDefaultKey_DiffViewMode options:0 context:(__bridge void*)[GIDiffContentsViewController class]];
+    [[NSUserDefaults standardUserDefaults] addObserver:self forKeyPath:GIUserDefaultKey_FontSize options:0 context:(__bridge void*)[GIDiffContentsViewController class]];
   }
   return self;
 }
 
 - (void)dealloc {
   [[NSNotificationCenter defaultCenter] removeObserver:self name:NSViewBoundsDidChangeNotification object:_tableView.superview];
 
+  [[NSUserDefaults standardUserDefaults] removeObserver:self forKeyPath:GIUserDefaultKey_FontSize context:(__bridge void*)[GIDiffContentsViewController class]];
   [[NSUserDefaults standardUserDefaults] removeObserver:self forKeyPath:GIDiffContentsViewControllerUserDefaultKey_DiffViewMode context:(__bridge void*)[GIDiffContentsViewController class]];
 }
 
@@ -227,13 +230,13 @@ - (Class)_diffViewClassForChange:(GCFileDiffChange)change {
     if ((change == kGCFileDiffChange_Untracked) || (change == kGCFileDiffChange_Added) || (change == kGCFileDiffChange_Deleted)) {
       return [GIUnifiedDiffView class];
     }
-    return self.view.bounds.size.width < kMinSplitDiffViewWidth ? [GIUnifiedDiffView class] : [GISplitDiffView class];
+    return self.view.bounds.size.width < kMinSplitDiffViewWidthEms * GIFontSize() ? [GIUnifiedDiffView class] : [GISplitDiffView class];
   }
   return mode > 0 ? [GISplitDiffView class] : [GIUnifiedDiffView class];
 }
 
-- (void)_updateDiffViews {
-  BOOL reload = NO;
+- (void)_updateDiffViewsForcingReload:(BOOL)forceReload {
+  BOOL reload = forceReload;
   for (GIDiffContentData* data in _data) {
     if (!data.diffView) {
       continue;
@@ -256,7 +259,7 @@ - (void)_updateDiffViews {
 
 - (void)viewDidResize {
   if (self.viewVisible && !self.liveResizing) {
-    [self _updateDiffViews];
+    [self _updateDiffViewsForcingReload:NO];
     [NSAnimationContext beginGrouping];
     [[NSAnimationContext currentContext] setDuration:0.0];  // Prevent animations in case the view is actually not on screen yet (e.g. in a hidden tab)
     [_tableView noteHeightOfRowsWithIndexesChanged:[NSIndexSet indexSetWithIndexesInRange:NSMakeRange(0, [self numberOfRowsInTableView:_tableView])]];
@@ -265,15 +268,16 @@ - (void)viewDidResize {
 }
 
 - (void)viewDidFinishLiveResize {
-  [self _updateDiffViews];
+  [self _updateDiffViewsForcingReload:NO];
   [_tableView noteHeightOfRowsWithIndexesChanged:[NSIndexSet indexSetWithIndexesInRange:NSMakeRange(0, [self numberOfRowsInTableView:_tableView])]];
 }
 
 // WARNING: This is called *several* times when the default has been changed
 - (void)observeValueForKeyPath:(NSString*)keyPath ofObject:(id)object change:(NSDictionary*)change context:(void*)context {
   if (context == (__bridge void*)[GIDiffContentsViewController class]) {
-    if (self.viewVisible) {
-      [self _updateDiffViews];  // This is idempotent
+    BOOL isFontSizeChange = [keyPath isEqualToString:GIUserDefaultKey_FontSize];
+    if (self.viewVisible || isFontSizeChange) {
+      [self _updateDiffViewsForcingReload:isFontSizeChange];  // This is idempotent
     }
   } else {
     [super observeValueForKeyPath:keyPath ofObject:object change:change context:context];

GitUpKit/Components/GIDiffContentsViewController.m

@@ -32,6 +32,12 @@
 @property(nonatomic, readonly, getter=isEmpty) BOOL empty;
 - (CGFloat)updateLayoutForWidth:(CGFloat)width;
 
+@property(nonatomic, readonly) CFDictionaryRef textAttributes;
+@property(nonatomic, readonly) CTLineRef addedLine;
+@property(nonatomic, readonly) CTLineRef deletedLine;
+@property(nonatomic, readonly) CGFloat lineHeight;
+@property(nonatomic, readonly) CGFloat lineDescent;
+
 @property(nonatomic, readonly) BOOL hasSelection;
 @property(nonatomic, readonly) BOOL hasSelectedText;
 @property(nonatomic, readonly) BOOL hasSelectedLines;

GitUpKit/Interface/GIDiffView.h

@@ -18,40 +18,67 @@
 #endif
 
 #import "GIPrivate.h"
+#import "GIAppKit.h"
 
-#define kTextFontSize 10
 #define kTextLineHeightPadding 3
 #define kTextLineDescentAdjustment 1
 
-CFDictionaryRef GIDiffViewAttributes = nil;
+const char* GIDiffViewMissingNewlinePlaceholder = "🚫\n";
 
-CTLineRef GIDiffViewAddedLine = NULL;
-CTLineRef GIDiffViewDeletedLine = NULL;
+@interface GIDiffView ()
 
-CGFloat GIDiffViewLineHeight = 0.0;
-CGFloat GIDiffViewLineDescent = 0.0;
+@property(nonatomic, assign) CGFloat lastFontSize;
 
-const char* GIDiffViewMissingNewlinePlaceholder = "🚫\n";
+@end
 
 @implementation GIDiffView
 
-+ (void)initialize {
-  GIDiffViewAttributes = CFBridgingRetain(@{(id)kCTFontAttributeName : [NSFont userFixedPitchFontOfSize:kTextFontSize], (id)kCTForegroundColorFromContextAttributeName : (id)kCFBooleanTrue});
+- (void)updateMetricsFromCurrentFontSize {
+  CGFloat newSize = GIFontSize();
+// This comparison is safe because the values being compared are both read from user defaults with no additional floating point operations.
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wfloat-equal"
+  if (newSize == _lastFontSize) {
+#pragma clang diagnostic pop
+    return;
+  }
+  _lastFontSize = newSize;
+
+  NSFont* font = [NSFont userFixedPitchFontOfSize:newSize];
+  if (_textAttributes) CFRelease(_textAttributes);
+  _textAttributes = CFBridgingRetain(@{(id)kCTFontAttributeName : font, (id)kCTForegroundColorFromContextAttributeName : (id)kCFBooleanTrue});
 
-  CFAttributedStringRef addedString = CFAttributedStringCreate(kCFAllocatorDefault, CFSTR("+"), GIDiffViewAttributes);
-  GIDiffViewAddedLine = CTLineCreateWithAttributedString(addedString);
+  CFAttributedStringRef addedString = CFAttributedStringCreate(kCFAllocatorDefault, CFSTR("+"), _textAttributes);
+  if (_addedLine) CFRelease(_addedLine);
+  _addedLine = CTLineCreateWithAttributedString(addedString);
   CFRelease(addedString);
 
-  CFAttributedStringRef deletedString = CFAttributedStringCreate(kCFAllocatorDefault, CFSTR("-"), GIDiffViewAttributes);
-  GIDiffViewDeletedLine = CTLineCreateWithAttributedString(deletedString);
+  CFAttributedStringRef deletedString = CFAttributedStringCreate(kCFAllocatorDefault, CFSTR("-"), _textAttributes);
+  if (_deletedLine) CFRelease(_deletedLine);
+  _deletedLine = CTLineCreateWithAttributedString(deletedString);
   CFRelease(deletedString);
 
   CGFloat ascent;
   CGFloat descent;
   CGFloat leading;
-  CTLineGetTypographicBounds(GIDiffViewAddedLine, &ascent, &descent, &leading);
-  GIDiffViewLineHeight = ceilf(ascent + descent + leading) + kTextLineHeightPadding;
-  GIDiffViewLineDescent = ceilf(descent) + kTextLineDescentAdjustment;
+  CTLineGetTypographicBounds(_addedLine, &ascent, &descent, &leading);
+  _lineHeight = ceilf(ascent + descent + leading) + kTextLineHeightPadding;
+  _lineDescent = ceilf(descent) + kTextLineDescentAdjustment;
+
+  [self setNeedsDisplay:YES];
+}
+
+// WARNING: This is called *several* times when the default has been changed
+- (void)observeValueForKeyPath:(NSString*)keyPath ofObject:(id)object change:(NSDictionary*)change context:(void*)context {
+  if (context == (__bridge void*)[GIDiffView class]) {
+    if ([keyPath isEqualToString:GIUserDefaultKey_FontSize]) {
+      [self updateMetricsFromCurrentFontSize];
+    } else {
+      XLOG_UNREACHABLE();
+    }
+  } else {
+    [super observeValueForKeyPath:keyPath ofObject:object change:change context:context];
+  }
 }
 
 - (void)_windowKeyDidChange:(NSNotification*)notification {
@@ -74,6 +101,7 @@ - (void)viewDidMoveToWindow {
 
 - (void)didFinishInitializing {
   _backgroundColor = NSColor.textBackgroundColor;
+  [[NSUserDefaults standardUserDefaults] addObserver:self forKeyPath:GIUserDefaultKey_FontSize options:NSKeyValueObservingOptionInitial context:(__bridge void*)[GIDiffView class]];
 }
 
 - (instancetype)initWithFrame:(NSRect)frameRect {
@@ -93,6 +121,12 @@ - (instancetype)initWithCoder:(NSCoder*)coder {
 - (void)dealloc {
   [[NSNotificationCenter defaultCenter] removeObserver:self name:NSWindowDidResignKeyNotification object:nil];
   [[NSNotificationCenter defaultCenter] removeObserver:self name:NSWindowDidBecomeKeyNotification object:nil];
+
+  [[NSUserDefaults standardUserDefaults] removeObserver:self forKeyPath:GIUserDefaultKey_FontSize context:(__bridge void*)[GIDiffView class]];
+
+  if (_textAttributes) CFRelease(_textAttributes);
+  if (_addedLine) CFRelease(_addedLine);
+  if (_deletedLine) CFRelease(_deletedLine);
 }
 
 - (BOOL)isOpaque {

GitUpKit/Interface/GIDiffView.m

@@ -20,14 +20,6 @@
 
 #if __GI_HAS_APPKIT__
 
-extern CFDictionaryRef GIDiffViewAttributes;
-
-extern CTLineRef GIDiffViewAddedLine;
-extern CTLineRef GIDiffViewDeletedLine;
-
-extern CGFloat GIDiffViewLineHeight;
-extern CGFloat GIDiffViewLineDescent;
-
 extern const char* GIDiffViewMissingNewlinePlaceholder;
 
 #endif