-
Notifications
You must be signed in to change notification settings - Fork 440
fix: Don't log if --json or --quiet
#4202
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
Merged
Changes from 48 commits
Commits
Show all changes
58 commits
Select commit
Hold shift + click to select a range
d9ac264
Don't log if `--json` or `--quiet`
Klaim 243d928
fixed LogHandler_History incorrect filtering of log records + added r…
Klaim bb89645
improved logging tests
Klaim 58dc097
json output will contain log records passing the log level filter (>=…
Klaim 9ca3a20
attempt to get some info from tests
Klaim 22a1177
fixed test expecting an error in cerr, looking into json instead
Klaim fe59cb8
LogHandler_History allows capture with clear
Klaim 5515fb1
output a potentially empty json if `--json` is used even if no log hi…
Klaim f529c23
console must capture and output json after error logging
Klaim dee6f12
(micro)mamba always reset the console whatever the way it returns
Klaim 4f3db35
tests: read json from process error (attempt)
Klaim e62c40e
do not output Console's JSON when command already outputs JSON or YAML
Klaim 5596d03
`--json` and `--quiet` are specially handled before creating the cont…
Klaim 91c65a8
keep "run --help" message when arguments parsing failed
Klaim 2f7c9ae
workaround compilers incorrectly requiring explicit member initializa…
Klaim 808be20
config sub-commands outputing json are merged with the normal json ou…
Klaim 1e752c7
removed incorrect code and comment
Klaim ce45264
let cli11 handle it's ownt errors but output where we want it to depe…
Klaim ee538ba
fix test json output erorr checks
Klaim 63ed7bd
fixup
Klaim 3fc7d69
partial text fix
Klaim cfeadc9
list sub-command's array of packages is now part of the returned json…
Klaim b6736ee
fixed: tools log-handlers not properly filtering log records
Klaim fafe861
adapt integration tests `list` calls assuming the result is a package…
Klaim b3b5110
fixed: `mamba create --dry-run --json` output 2 json objects
Klaim 139beec
fixed: `list` subcommand tests guaranteed to return an array even if …
Klaim 7eeed5a
fixed: `mamba env list --json` outputs 2 json objects
Klaim 30de13b
fixed: `mamba env export|list --json` outputing 2 json objects
Klaim b7f20cf
fix: mark `LogHandler_History::is_started` inline in header
jjerphan 42aef3d
Make linters happy
jjerphan 5e72263
merge origin/main + adaptations
Klaim a8753d3
log handlers are now comparable to check if they refer to the same in…
Klaim 0fce75f
fixed: logging backtrace changes were not persisted in logging parame…
Klaim 1394350
tweak/simplification
Klaim a5620e8
Merge remote-tracking branch 'origin/main' into no-logging-json-quiet
Klaim b8b7c12
fixed: incorrect JSON pointer formation from object members names
Klaim b962248
mamba tests: expect the "log_history" field in json output
Klaim 4a78575
repoquery subcommand json output now goes through the general output …
Klaim 38c3bc9
support version output in json
Klaim 7c7d66a
updated comment in tests
Klaim 17e9a06
fixed: hide_secrets regex matching with empty names
Klaim a98b70e
Merge remote-tracking branch 'origin/main' into no-logging-json-quiet
Klaim e495113
increased probably too big timeout
Klaim 007e13a
formatting
Klaim 9d8b7bd
formatting
Klaim 240d53d
removed unclear comment
Klaim daeb4ee
improved log record filtering clarity (addressing reviews)
Klaim 3e21106
Merge branch 'main' into no-logging-json-quiet
Klaim da56561
fix typo
Klaim 2146be8
fix typo
Klaim 3b3ebae
Revert "increased probably too big timeout"
Klaim ace8813
rewrote basic_backtrace::push_if_enabled to take an invocable used if…
Klaim 395cbcb
(micro)mamba: rewrote options intiialization for clarity
Klaim 05c46ac
removed comments that is not up to date
Klaim ba8cfd5
appease warnings that assumes developers ignores of the concept of "s…
Klaim fa0f430
formatting
Klaim fd5a8e8
Merge remote-tracking branch 'origin/main' into no-logging-json-quiet
Klaim 855eb58
Merge branch 'main' into no-logging-json-quiet
Klaim File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.
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 am still uncomfortabe with this API and the move below. Given that this function is called here and here only, that the LogRecord is passed by value and is not used after the call to
push_if_enabled, I think it is safe to accept it by rvalue reference here instead of lvalue reference.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.
When this function returns
false, in both usage we move the record in another container, so I can't just take by r-value-reference without at least returning the object again so that it can be used by the caller, otherwise at the call point it would look like we move the object twice, which is even weirder.My goal here is to have this algorithm written only once, as having the same logic in both place lead to some annoying errors while developing. But I also agree it's quite weird api.
I'll try to find another way to achieve that goal with a nicer api. I was thinking maybe something in that direction:
The passing of the same object to the lambda is a bit weird but maybe less weird than this current api?
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 was thinking of having
push_to_backtracereturning the log record if not pushed (can be an optional, or a pair<bool, LogRecord>), but that would make the syntax heavier. Passing a lambda as you suggest seems the best option in the end, I don't find it weird to pass the same object to the lambda; and it is not that obvious at the call site.