Add quiet option #167
Add quiet option #167
Conversation
There was a problem hiding this comment.
Thanks @soffes! Looks good. I didn't think that people might not want the summary output 😄
Personally I use the include feature to bring together many different partial spec files, so it's useful to see what actually ended up in the final spec.
If we added a --verbose option later, that would probably fight with --quite. Maybe it's even worth locking the current output behind a verbose flag. What do you think? I would like to keep the current output as default though. Is it just annoying or are there technical considerations for removing it?
Sources/XcodeGen/Logger.swift
Outdated
|
|
||
| // MARK: - Properties | ||
|
|
||
| /// Should success messages be suppressed |
There was a problem hiding this comment.
Seems it suppresses info not success messages
Sources/XcodeGen/main.swift
Outdated
| } | ||
| } | ||
|
|
||
| command( | ||
| Option<String>("spec", "project.yml", flag: "s", description: "The path to the spec file"), | ||
| Option<String>("project", "", flag: "p", description: "The path to the folder where the project should be generated"), | ||
| generate) | ||
| .run(version) | ||
| Flag("quiet", flag: "q", description: "Suppress printing of success messages", default: false), |
There was a problem hiding this comment.
Seems it suppresses info not success messages
Sources/XcodeGen/Logger.swift
Outdated
| /// - parameter message: the message to log | ||
| func success(_ message: String) -> Never { | ||
| print(isColored ? message.green : message) | ||
| exit(0) |
There was a problem hiding this comment.
We might print multiple success messages. No need to exit here
|
Could you add documentation to Readme as well? |
Sources/XcodeGen/Logger.swift
Outdated
| // XcodeGen | ||
| // | ||
| // Created by Sam Soffes on 11/21/17. | ||
| // |
There was a problem hiding this comment.
Could you remove header. I've removed them all in master, and they are all removed automatically now in make format_code
|
Made the changes your requested! I tested this locally since there aren't tests for the output of the command line tool (yet!). I was thinking about |
Sources/XcodeGen/Logger.swift
Outdated
| /// - parameter message: the message to log | ||
| func fatal(_ message: String) -> Never { | ||
| print(isColored ? message.red : message) | ||
| exit(1) |
There was a problem hiding this comment.
Seems a bit weird to put this kind of logic in a Logger? You wouldn't really expect the program to exit after logging a fatal error, the user may want to do some extra cleanup after logging.
There was a problem hiding this comment.
Fair. I was just excited to use -> Never. I'll remove. I agree it's weird for the logger to exit.
Sources/XcodeGen/main.swift
Outdated
| print("Parsing spec failed: \(error.description)".red) | ||
| exit(1) | ||
| logger.fatal("Parsing spec failed: \(error.description)") | ||
| exit(1) |
There was a problem hiding this comment.
Every time I regenerate the Xcode project it changes it back to tabs 🙄
Sources/XcodeGen/Logger.swift
Outdated
|
|
||
| // MARK: - Logging | ||
|
|
||
| /// Log a fatal message and exit with status code 1 |
There was a problem hiding this comment.
This is why I stay away from doc comments as they always get out of date. 😄 In this case, it mentions status code here (and in success).
If this doesn't exit anymore, we could also rename this to error.
Also if we wanted to remove all the duplicate exit(1) in the main file, a simple helper function there called fatalError(_ message: String) could log an error and then exit
There was a problem hiding this comment.
Sorry for not seeing this before :)
There was a problem hiding this comment.
fatalError(_ message: String) good use case for -> Never @soffes 😜
|
@yonaskolb thanks for your time on this! Sorry for all of the back and forth! |
|
Thank YOU :) |
This option will suppress success messages. A new
Loggerstruct is added to theXcodeGentarget that encapsulates printing, exiting, and the Rainbow logic.I only added a Commander flag for
--quiet. If we added a Commander interface, it would be trivial to remove colored output too if that is desired. I didn't do this since I couldn't decide on a name. Also, I figured if you were in a situation where you didn't want colored output, you might also want to suppress emojis.