vscode icon indicating copy to clipboard operation
vscode copied to clipboard

Add new/missing defmt log format options

Open bugadani opened this issue 10 months ago • 7 comments

Source is https://github.com/probe-rs/probe-rs/blob/08cc1456dd54e8df7a7afe34b0ba0f8345506752/probe-rs/src/bin/probe-rs/util/rtt.rs#L124-L145

This PR depends on a 0.24 release

bugadani avatar Apr 16 '24 20:04 bugadani

Why is channelName a number?

Copy-paste-forget bug, sorry

Can we not somehow incorporate it into

This is a side effect of what cargo embed does: there is an outer option, that serves as a default for unconfigured channels, and inner ones have the same option to override this default. This is probably excessive...

bugadani avatar Apr 16 '24 21:04 bugadani

@bugadani I just remembered ... we used to have a channel name, and then removed it, because it can be specified in the target application when the channels are configured. Since we would have to arbitrarily choose which one takes priority, it seemed to make more sense to just let the naming happen on the rust side, and then name the client channels based on that.

noppej avatar Apr 16 '24 21:04 noppej

@bugadani I'm not sure I like having the outer and inner option. It adds code complexity and maintenance, for something that the user configures once in a launch.json file, and probably very seldom modified after that.

Does it really add value to have both?

noppej avatar Apr 16 '24 21:04 noppej

Honestly I don't know, but the option is there in Rust - with the same question attached. I guess we can sack it from cargo embed, too, and forget the idea of a default set of configuration altogether.

bugadani avatar Apr 16 '24 21:04 bugadani

I don't want to propose changing cargo-embed ... it will impact existing users. I am just suggesting that we don't bring the unnecessary complexity to the dap-server.

Especially since there can only be one defmt channel, what is the point of having an outer setting? Either set it for the one channel you have, or don't. Or am I missing something?

noppej avatar Apr 16 '24 21:04 noppej

Well then I just added a bunch of code to delete it :)

cargo embed's config format has already been changed in not a small way, so I think twisting it once more shouldn't be too bad. Also, as you've said, this should be a one time annoyance.

bugadani avatar Apr 16 '24 21:04 bugadani

Last commit enables squigglies on typos:

image

Relevant (but not really helpful?) docs: https://json-schema.org/draft/2020-12/json-schema-core#section-10.3.2.3

bugadani avatar May 11 '24 10:05 bugadani