Handle input with no format specifiers in `Formatter`

AtkinsSJ commented 2 years ago

Hi! This is my first contribution to a spec like this so while I've done my best to follow the guidelines, there are probably things I am getting wrong. Let me know and I'll try and sort them out as soon as possible.

This fixes #204.

As noted in that issue, several functions call Formatter without checking that a format specifier is present, but the Formatter algorithm assumed this was never the case. When called in this way, Formatter would use the result variable without defining it.

This patch makes the following changes:

Add a step 4 to the Formatter algorithm to explicitly handle a lack of format specifier.
Correct the indentation of what was step 3.8 of Formatter, so it is now step 6.
Remove what was step 4 from Formatter, since this is now redundant.
Remove step 5 from the Logger algorithm, since this is now redundant.
Add a new step 1 to Formatter, which returns early if only one argument was provided.

[ ] At least two implementers are interested (and none opposed):
- @terinjokes
- @domfarolino
[ ] Tests are written and can be reviewed and commented upon at:
- No tests added. This is a correction to the spec, to match what was already intended.

(See WHATWG Working Mode: Changes for more details.)

Preview | Diff

AtkinsSJ commented 2 years ago

Realised another small change I forgot to make, will update this when I'm back at my desk.

AtkinsSJ commented 2 years ago

New changes: Remove a redundant step from Formatter, and make the commit message clearer.

AtkinsSJ commented 2 years ago

Just realised in the shower: Formatter also doesn't handle the case of being called with only 1 arg, which again happens when called from trace(), group() and groupCollapsed().

New changes: Handle that case in Formatter.

foolip commented 2 years ago

@domfarolino @robertkowalski @terinjokes can any of you take a look? (@AtkinsSJ just asked on the Matrix channel.)

terinjokes commented 2 years ago

I can take a look this evening.

AtkinsSJ commented 2 years ago

@terinjokes Any thoughts on this so far?

terinjokes commented 2 years ago

LGTM. @domfarolino ?

domfarolino commented 2 years ago

LGTM. @domfarolino ?

Yep I think this LGTM as well, but I'll defer to you @terinjokes since you got to this first!

AtkinsSJ commented 2 years ago

Just remembered this PR again, and rebased it on the main branch. Is there anything I still need to do to get verified as a WHATWG participant? I registered for that quite a while ago.

domfarolino commented 1 year ago

Looks like you already signed the participant agreement and I've gone ahead and verified you as well. Looks like the build is acting up so let me close this and re-open it to see if that helps.

domfarolino commented 1 year ago

Thanks a lot @AtkinsSJ!

AtkinsSJ commented 1 year ago

Thanks for the merge! :^)

whatwg / console

Handle input with no format specifiers in `Formatter` #205