-
Notifications
You must be signed in to change notification settings - Fork 234
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged iosMath and MacOSMath. #30
Conversation
Add Chinese support.
It seems that GitHub does not recognize |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor code review feedback.
// | ||
// Created by 安志钢 on 17-01-08. | ||
// | ||
// |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On the top of any new .h or .m file that you add could you please add:
//
// This software may be modified and distributed under the terms of the
// MIT license. See the LICENSE file for details.
//
@property (nonatomic, nullable) UIColor* textColor; | ||
#else | ||
@property (nonatomic, nullable) NSColor *textColor; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of doing this everywhere do this:
In a separate header file have this:
#if TARGET_OS_IPHONE
#import <UIKit/UIKit.h>
#else
#import <AppKit/AppKit.h>
#endif
#if TARGET_OS_IPHONE
#define MTColor UIColor
#else
#define MTColor NSColor
#endif
See SpriteKitBase.h
for how Apple does it.
NSString *reqSysVer = @"6.0"; | ||
NSString *currSysVer = [UIDevice currentDevice].systemVersion; | ||
#else | ||
// This is Darwin version of 10.8.x. | ||
NSString *reqSysVer = @"12.0"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
None of this code is needed. iosMath supports iOS 6+ only. This code was present to make it work on iOS 5. You can remove this function an the place where it is used.
UIBezierPath* path = [UIBezierPath bezierPath]; | ||
#else | ||
NSBezierPath* path = [NSBezierPath bezierPath]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do this one same as MTColor
as well
And then add a category on NSBezierPath
which adds the method addLineToPoint
, then we can get rid of all these #if #else
@import UIKit; | ||
#else | ||
@import AppKit; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to fail on the Travis build. Does something need to be setup to get the preprocessor macros to work correctly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found the reason. I didn’t include TargetConditionals.h, nor import Foundation/Foundation.h, so the symbol TARGET_OS_IPHONE
is always 0. I’ve fixed this in new pull request (#31).
[self setNeedsLayout]; | ||
#else | ||
[self setNeedsLayout:YES]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just define a method
- (void)setNeedsLayout:(BOOL) val {
[self setNeedsLayout]
}
for the iPhone side. Then you can avoid all these #if #else
.
@@ -98,38 +140,73 @@ - (void)setLatex:(NSString *)latex | |||
if (error) { | |||
_mathList = nil; | |||
_error = error; | |||
_errorLabel.text = error.localizedDescription; | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#if TARGET_OS_IPHONE
_errorLabel.text = error.localizedDescription;
#else
_errorLabel.stringValue = error.localizedDescription;
#endif
is a lot more readable.
- (void)drawRect:(CGRect)rect | ||
#else | ||
- (void)drawRect:(NSRect)dirtyRect |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use similar trick as colors.
#if TARGET_OS_IPHONE | ||
rect | ||
#else | ||
dirtyRect |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is unnecessary, call both the parameters as rect.
- (void) layoutSubviews | ||
#else | ||
- (void)layout |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe just define a separate method:
- (void) layout
{
[self layoutSubviews]
[super layout]
}
Thanks for the awesome PR! It is great to make iosMath work on MacOS. I have some mostly minor comments. Please fix and update. For some reason it is not building on Travis, if you can figure out why it is building the MacOS version on the iOS build it would be great. |
Sure, I'll update my PR, maybe later today. Thank you! |
Merge new commit from upstream.
Merged iOS and Mac, more cleaner version.
Merge local copy and remote copy.
I’ve merged iosMath and MacOSMath.
Notes:
iosMathExample may NEED test. I don’t have pod installed on this mac, so I did not check that. (There shouldn’t be any problem.)
UIKit/AppKit API compatibility is achieved by using preprocessor
TARGET_OS_IPHONE
.Podfile is updated because I added MacOSMath.xcodeproj.
workspace
andproject
have to be explicitly directed toiosMath.xcodeproj
.In file
MTMathDisplay.h/m
, method- (void)debugQuickLookObject
is commented out by preprocessor if the library is used on Mac.In file
MTFont.m
class method+ (NSBundle*) fontBundle
, I use main bundle instead of class bundle. This should be changed to class bundle.File
README.md
is NOT updated.Chinese support in PR Chinese characters support added. #29 is NOT added in case of further improvements of Chinese support.
Maybe the name
iosMath
can be changed intoMathRender
;-). No,MathRender
is not a good name.