Organised Subcommand Selection, Extended cfg writing abilities, Added cfg option to disable timestamps. #259
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
( Removes need for min_argvs) Moved cfg subcommand initialisation to separate function. Added some notes on how to add more subcommands.
Stop testing for this branch.
leotrs
suggested changes
Aug 10, 2020
…nto extend_subcmds
PgBiel
reviewed
Aug 11, 2020
PgBiel
reviewed
Aug 11, 2020
Co-authored-by: Pg Biel <9021226+PgBiel@users.noreply.github.com>
Co-authored-by: Pg Biel <9021226+PgBiel@users.noreply.github.com>
PgBiel
reviewed
Aug 11, 2020
Thank you, @PgBiel ! Co-authored-by: Pg Biel <9021226+PgBiel@users.noreply.github.com>
eulertour
reviewed
Aug 12, 2020
As per @eulertour 's recommendation.
…nto extend_subcmds
leotrs
approved these changes
Aug 12, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
List of Changes
cfgsubcommand initialisation to a separate function; documented how a subcommand is selected.cfg writedefault.cfgdefault.cfgto choose whether or not timestamps should be present in the logs.Motivation
For adding an option to disable timestamps and disabling them when testing log file writing.
When discussing the efficacy of Implement Logging of Terminal Output to a .log file. #180, @huguesdevimeux recommended that an option should be added which disables logging timestamps along with the other data that is logged, allowing for word-for-word tests.
This was to increase the accuracy of future tests that may use the log-to-file feature.
For adding input validation for
cfg writeAs the
cfgsubcommands all depend on the contents of thecfgfiles, any changes to them will impact how they work; in particular, adding an entry would mean modifyingcfg writeandcfg showsuch that it could write/display such an entry if run.This could be done much easier if there was a general way to validate input for
cfg writeand recognise the datatypes that each entry should be.For moving
cfgsubcommand initialisation to a separate function and documenting how a subcommand is selected.Issue Add Manim Project Creating Subcommand #244 requests the addition of a new subcommand,
manim init. Implementing this would presumably be similar to howmanim cfgwas implemented. To make it clearer and easier as to how exactly a subcommand has to be implemented, thecfgsubcommand initialisation has been moved to a separate function, and the subcommand selection process has been documented as docstrings.Testing Status
Pytest exited with no errors.
Further Comments
@azarzadavila had mentioned that they would like to work on #244 , and I had suggested that they wait until I take a PR implementing some changes to subcommand selection. This is that PR, so once it's merged, they can feel free to start working on #244 :)
Acknowledgement