-
-
Notifications
You must be signed in to change notification settings - Fork 60
Add CLI support #12
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
base: master
Are you sure you want to change the base?
Add CLI support #12
Conversation
Thank you for the submission @An-GG! I'll have a look at this in the next few days. On first pass, the printf bits in the logging with the ansii escape chars were initially a red flag for me - maybe remove those if they were just there for debugging. Looking further, it looks like some interesting stuff will have a deeper look soon :) |
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.
Thanks for taking the time to do this @An-GG! If you just did this as a one off, and don't feel like / are too busy to follow up on this, just let me know.
Cheers
|
||
- (void)logError:(NSString*) message | ||
{ | ||
printf("\x1b[1;91m%s\x1b[0m\n", message.UTF8String); |
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 think we should remove these printfs
. I believe these are here to show output when running from the command line version - they might be better to just be in the logMessage function and not do the colours as we're assuming the colour scheme of the terminal.
INFOPLIST_FILE = Info.plist; | ||
INSTALL_PATH = "$(HOME)/Applications"; | ||
MACOSX_DEPLOYMENT_TARGET = 10.13; | ||
OTHER_CODE_SIGN_FLAGS = "--deep"; |
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 don't know what that does - I did a quick google search and it's not super clear, can you let me know?
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.
honestly i dunno, all I remember is that the project would not build in Xcode, there was some recommendation given by Xcode to fix the issue and I hit apply, and then it built (and then these edits showed up in the git diff
)
- are you able to build with this change reverted?
- are you able to build with
xcodebuild
from the command line? (its possible I made this change to fix command line only builds)
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 don't think it's a problem, I was just curious about it. Once it's merged and runs though the github action, if its an issue I'll have a deeper look.
} | ||
} | ||
return nil; | ||
} |
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 cool, and looks like good fun :) but did you try getopt? https://pubs.opengroup.org/onlinepubs/000095399/functions/getopt.html has a few safety bits that might be nice.
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.
ah that's neat.
looks powerful but gonna be honest personally I didnt find the interface of getopt all that intuitive
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.
No, it isn't intuitive at all :) it just covers some edge cases, and saves you from having to reimplement a few things. It does fall short in some areas too, but it is a useful tool to have at your disposal 🛠️
Hi Rob, happy to help you merge this PR. Should be able to get back to you on these concerns tonight. |
|
probably not the right place for this but great article. i learned something new |
This adds support for using the built executable from the command line. If valid command line arguments are detected, the file is extracted and the program exits without launching graphical windows.
Additionally, some improvements / organized some stuff: