Skip to content
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

[RFC] display hints (:x, :b, :a) #270

Closed
japaric opened this issue Nov 24, 2020 · 9 comments
Closed

[RFC] display hints (:x, :b, :a) #270

japaric opened this issue Nov 24, 2020 · 9 comments
Assignees
Labels
breaking change fix / feature / improvement involves a breaking change and needs to wait until next minor version status: needs design This feature needs design work to move forward type: enhancement Enhancement or feature request
Milestone

Comments

@japaric
Copy link
Member

japaric commented Nov 24, 2020

this is a feature proposed for version 0.2.0.
(this feature cannot be implemented in the 0.1.x series because it involves breaking changes in the wire format, parser and/or decoder.)
the design is not final and it's still being reviewed by the knurling team.

1. optional type annotation

(0.2's {} === 0.1's {:?})

In v0.2.0, the empty format parameter ({}) will be accepted. It will be understood as 'this is some impl Format value'.

// 0.1.x
info!("{:?}", 42);

// 0.2.x -- equivalent
info!("{}", 42);

it will still possible to specify the type using the =$type syntax to improve encoding size

// typed with no hints
info!("{=u8}", 42);

// typed with hints
info!("{=u8:x}", 42);

when format parameter includes both type and hint the type must come first: :x=u8 is not valid syntax
(this restriction could be lifted in the future)

this feature combined with feature (3) will ease migration from the log! macro to the defmt macros:
if {:$hint} is a valid core::fmt format string then it's also a valid defmt format string

2. default to Display-like format (fallback to Debug)

in 0.2.0 types, like strings, will be formatted "Display-like", that is without double quotes and escaped characters

info!("{}", "hello\tworld");
// -> INFO hello    world

if a type has no Display-like formatting option, it will formatted in a Debug-like fashion

info!("{}", [0, 1]);
// -> INFO [0, 1]

In 0.1.x this was not well specified

3. display hints

syntax

"display hints" can be tacked onto format parameters to tell a defmt printer that you want a value to be formatted (on the host side) in a certain way
as this is a hint, a printer (the host) may choose to ignore the hint

// `Format` value
info!("{:x}",    42); // -> INFO 0x2a

// typed value
info!("{:u8:x}", 42); // -> INFO 0x2a

the hint propagates inwards and applies to struct / enum fields (recursively)

struct S { x: u8, y: u8 }

info!("{:x}", S { x: 32, y: 33 });
// -> INFO S { x: 0x20, y: 0x21 }

initial support

initially we plan to support the following hints:

  • x (lowercase), X (uppercase) hexadecimal
  • b binary
  • a ASCII
  • ? Debug

the ASCII hint applies to byte arrays and slices.
when the printer sees this hint it should format the bytes using Rust's byte string literal notation.
Example:

info!("{:a}", [104, 105, 127]);
// -> INFO b"hi\x7f"

? formats the value in a Debug-like manner.
For the str type this means the string will be double quoted and escaped

info!("{}", "hello\tworld");
// -> INFO hello    world

info!("{:?}", "hello\tworld");
// -> INFO "hello\tworld"

forward compatibility

to allow for the possibility of adding more display hints in the v0.2.x series, defmt tools should ignore display hints they don't understand instead of throwing runtime errors.

// defmt 0.2.0          probe-run 0.2.0
info!("{:x}",   42); // -> INFO 0x2a

// defmt 0.2.1          probe-run 0.2.0
info!("{:04x}", 42); // -> INFO 42

macros will validate display hints and reject unknown hints at compile time

// defmt 0.2.0
info!("{:04x}", 42);
//~ error `04x` is an unknown display hint

#[derive(Format)]

this macro will apply the :? display hint to all struct / enum fields.
this way string fields will be printed with double quotes and escaped characters

#[derive(Format)]
struct S { x: &'static str }

info!("{}", S { x: "hi" });
// -> INFO S { x: "hi" }

this makes the behavior closer to the behavior of #[derive(Debug)]

impl notes

// existing enum in `decoder` that was renamed
enum ArgKind {
   Uxx(u64),
   // ..
}

// replaces the 0.1.x `enum Arg`
struct Arg {
    kind: ArgKind,
    hint: Hint,
}

#[non_exhaustive] // for forwards compatibility
enum Hint {
    Hexadecimal { uppercase: bool },
    Binary,
    Ascii,
    Unknown(String), // not understood in this version
}

unresolved questions

  • pick default hexadecimal & binary display format

a. 0x1234_5678 / 0b1111_0000 (with 0x suffix and grouping)
pros:

  • valid Rust syntax
  • easier to read large/long values
    cons:
  • may not be accepted by other programs (calculators)

b. 0x12345678 / 0b11110000 (with 0x suffix and no grouping)
pros:

  • valid Rust syntax

  • all programs understand this hexadecimal / binary format
    cons:

  • harder to read large/long values

  • is the type / display order fixed? or is :x:u8 accepted?

  • how to switch to Debug-like format (e.g. for strings)? :? or :#?

  • change type syntax to something less ambiguous? e.g. =u8 =u8:x

  • struct inter-operation

struct S { x: &'static str }
info!("{}", S { x: "y" });
// should this be
// -> INFO S { x: y }
// OR
// -> INFO S { x: "y" }
  • should macros reject unknown display hints?

changelog

2020-11-25

added:

  • pros & cons for different hex / bin format
  • :? is the "Debug"-like display hint
  • =u8 (instead of :u8) for type information
  • macros reject unknown display hints
  • derive(Format) applies :? hint to struct / enum fields
@japaric japaric added type: enhancement Enhancement or feature request status: needs design This feature needs design work to move forward labels Nov 24, 2020
@japaric japaric added this to the v0.2.0 milestone Nov 24, 2020
@japaric
Copy link
Member Author

japaric commented Nov 24, 2020

this feature cannot be implemented in the 0.1.x series because it involves breaking changes in the wire format, parser and/or decoder.

to expand on this: existing probe-run versions like 0.1.6 expect a certain format string syntax. if we extend the format string syntax in defmt 0.1.x then probe-run 0.1.6 will fail to decode the defmt data. The failure mode is silent failure: the log stream stops and the application keeps running.

for that reason any change in the format string syntax needs to be done in defmt 0.2.0. That defmt version will work with a probe-run 0.2.0, which will not work with defmt 0.1.0: the tool will reject apps that use defmt 0.1 -- it will refuse to flash them and tell you to use a different probe-run version

@japaric
Copy link
Member Author

japaric commented Nov 24, 2020

initial support

note that the goal of this RFC is not to list upfront all the possible display hints but rather specify the feature in a way that let us add new display hints without those additions being breaking changes. That way we can add display hints in the 0.2.x series

@Dirbaio
Copy link
Contributor

Dirbaio commented Nov 24, 2020

syntax

The prosposed syntax feels somewhat ambiguous...?

  • If I have {:foo}, is foo supposed to be a type or a hint? There's no way to know other than checking foo against a list of known types/hints.
  • What orders are allowed? {:hint:type} or {:type:hint}, or both?

Alternative 1: join both into string, and require the hints to be "prefixes" to the type.

info!("{}", 42);
info!("{:u8}", 42);
info!("{:x}", 42);
info!("{:xu8}", 42);
  • good: more terse, faster to type
  • bad: harder to parse unknown hints for forwards-compat
  • bad: {:istr} is istr, or str with format hint i? :(

Alternative 2: specify type with one special char, hint with another. For example using : for hints, = for types:

info!("{}", 42);
info!("{=u8}", 42);
info!("{:x}", 42);
info!("{:x=u8}", 42);
info!("{=u8:x}", 42);
  • good: separates the concept of "type" and the concept of "format hint" very clearly
  • good: easy to parse unknown hints
  • bad: very incompatible
    • : for types, = for hints -> very incompatible with std fmt
    • : for hints, = for types -> very incompatible with defmt 1.x.
    • if I had to choose, I'd prioritize std fmt compatibility, because:
      • long term more people will be migrating to defmt 2 from std than from defmt 1
      • having special different syntax for the concept of "type" makes it clear that it's a very different concept, makes defmt easier to learn IMO.

to allow for the possibility of adding more display hints in the v0.2.x series, defmt tools should ignore display hints they don't understand instead of throwing errors.

Just to clarify, this means the decoder ignores unknown hints, but the macros still reject unknown hints?

pick default display format

👍 to the prefix: removes ambiguity and makes pasting to other tools do the right thing.
👎 to the grouping: it'll be annoying if you want to paste values to some calculator/language that doesn't support them.

how to switch to Debug-like format (e.g. for strings)?

I'd say format hint ?, to be consistent with std.

How does it interact with structs? my guess is like this:

struct S { x: &str }

info!("{}", S { x: "hello!" });
// -> INFO S { x: hello! }
info!("{:?}", S { x: "hello!" });
// -> INFO S { x: "hello!" }

@Dirbaio
Copy link
Contributor

Dirbaio commented Nov 24, 2020

the ASCII hint applies to byte arrays and slices.

Printing quoted strings is also useful for str though. This somewhat overlaps with ?, but it feels weird to me that if you want a quoted string from a [u8] you need to use a and from a str you need to use ?

@japaric
Copy link
Member Author

japaric commented Nov 24, 2020

The prosposed syntax feels somewhat ambiguous...?

we'll like to ease migration from log and have some level of inter-op between defmt and log so I think we'd like to use :hint syntax (with the leading semicolon). That way the format specifier {:x} works both with defmt and log.

the syntax for the type is not set in stone. prefixing it with a different symbol, e.g. =u8 seems reasonable.

as a side note we also considered moving the type info to the arguments, e.g. using type ascription syntax:

// alternative
info!("{:x}", x: u8);
// this proposal
info!("{:u8:x}", x);

but this is incompatible with the "bitfield range" type: {:4..8}

Just to clarify, this means
the decoder ignores unknown hints,

yes

but the macros still reject unknown hints?

this is unspecified. as far as forward compatibility is concerned I think it should be possible to have macros reject unknown hints at compile time. the main compatibility problem is: using a newer defmt patch version with an old probe-run (that supports the defmt major.minor version). as long as the probe-run's parser ignores unknown hints we should be fine; it should be OK if the macros' parser errors on unknown hints.

How does it interact with structs?

good question

// -> INFO S { x: hello! }

this looks odd to me because in my mind S { .. } is already Debug-like format so inner fields shouldn't use Display-like format.
I would vote for structs / enums are always (recursively) Debug-like and the :? hint has no effect on them

@Dirbaio
Copy link
Contributor

Dirbaio commented Nov 24, 2020

it should be OK if the macros' parser errors on unknown hints.

Yeah, IMO it's the best thing. Macros shouldn't silently ignore errors, otherwise you can typo your format hint and waste time trying to figure out why is it doing nothing.

this looks odd to me because in my mind S { .. } is already Debug-like format so inner fields shouldn't use Display-like format.
I would vote for structs / enums are always (recursively) Debug-like and the :? hint has no effect on them

Yeah this looks odd to me too, but it's what current defmt does. It makes sense to print all structs quoted (the generated Format impl could just put {:?} everywhere)

@japaric
Copy link
Member Author

japaric commented Nov 25, 2020

I have incorporated the feedback into the RFC (issue description)

the hint propagates inwards and applies to struct / enum fields (recursively)

we need to specify what's the behavior of "hint collision":

struct S { x: u8 }
impl Format for S {
    fn format(&self, f: &mut Formatter) {
        write!(f, "{:b}", self.x);
    }
}
// does this use hexadecimal notation or binary notation?
info!("{:x}", S { x: 16 });

the core::fmt behavior is to ignore the outer format specifier:

impl fmt::Debug for S {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        write!(f, "S {{ x: {:b} }}", self.x)
        //                   ^ cannot be overriden
    }
}
// the `x` specifier is ignored
println!("{:x?}", S { x: 16 }); // -> S { x: 1000 }

@jonas-schievink
Copy link
Contributor

Using the innermost specifier sounds like a reasonable thing to do

@japaric japaric added the breaking change fix / feature / improvement involves a breaking change and needs to wait until next minor version label Nov 27, 2020
@japaric japaric modified the milestone: v0.2.0 Nov 27, 2020
@japaric japaric self-assigned this Dec 16, 2020
bors bot added a commit that referenced this issue Dec 18, 2020
313: add display hints e.g. :x, :b, :a r=japaric a=japaric

**breaking-change** this changes the format string syntax as specified in RFC #270 

``` rust
// OLD syntax: will be rejected
info!("{:u8}", 42);

// NEW syntax: types are prefixed by '='; display hints are prefixed by ':'
info!("{=u8:x}", 42);

// Note that this works too and is now in line with the Rust std formatting syntax
info!("{:x}", 42);
```

Display hints that have been implemented:
- Binary (`:b`), e.g. `0b1010`
- Upper case hexadecimal (`:X`), e.g. `0xA0`
- Lower case hexadecimal (`:x`), e.g. `0xa0`
- ASCII (`:a`), e.g. `b"he\xffllo` (works on byte slices `[u8]`)
- Debug (`:?`), e.g. `"Hello"` (works on strings)

TODO
- [ ] update book and API doc

FIXME
- [ ] `derive(Format)` on structs that contains string fields formats those fields "Display"-style (w/o double quotes)

(may not be necessary to block this PR on that fix though)

Co-authored-by: Jorge Aparicio <jorge.aparicio@ferrous-systems.com>
@japaric
Copy link
Member Author

japaric commented Dec 18, 2020

implemented in #313

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change fix / feature / improvement involves a breaking change and needs to wait until next minor version status: needs design This feature needs design work to move forward type: enhancement Enhancement or feature request
Projects
None yet
Development

No branches or pull requests

3 participants