-
Notifications
You must be signed in to change notification settings - Fork 42
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
update the command line output to behave better #2726
Conversation
for more information, see https://pre-commit.ci
|
|
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
@trevorhardy can you check this again? |
Pulled down the new commits (7fbb609) and
|
7753a71
to
88903f8
Compare
for more information, see https://pre-commit.ci
@trevorhardy try again? |
@phlptp, no luck. The clone output JSON does now have |
@trevorhardy try again? |
for more information, see https://pre-commit.ci
That did it! This is the clone output:
|
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 haven't comprehensively tested this but I'm not aware of bugs at this time.
Fixes #2731 |
opt->check([](const std::string& fname) { | ||
static const std::set<std::string> validExt = { | ||
".ini", ".toml", ".json", ".INI", ".JSON", ".TOML"}; | ||
auto ext = std::filesystem::path(fname).extension().string(); |
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.
Should we just make the extension lowercase, so we only need to list one variant in the list of valid extensions?
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 thought about that but it seemed like it would be more efficient to just let the set deal with it, rather than call an additional function every time. I tried routing it through the CLI validator that does that but it had some annoying side effects so I went away from that option.
Co-authored-by: Ryan Mast <[email protected]>
Summary
If merged this pull request will adjust the output flag for recorders/clone and add capture flag for clone
Proposed changes