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

ABNF for sf-date #2540

Closed
Jxck opened this issue May 17, 2023 · 6 comments
Closed

ABNF for sf-date #2540

Jxck opened this issue May 17, 2023 · 6 comments

Comments

@Jxck
Copy link

Jxck commented May 17, 2023

according to "Parsing a Date"

https://github.com/httpwg/http-extensions/blob/main/draft-ietf-httpbis-sfbis.md#parsing-a-date-parse-date

Let output_date be the result of running Parsing an Integer or Decimal ({{parse-number}}) with input_string.

align ABNF to this line, I think sf-date should be like below

- sf-date = "@" ["-"] 1*15DIGIT
+ sf-date = "@" sf-integer
@reschke
Copy link
Contributor

reschke commented May 17, 2023

Not convinced. It adds a level of indirection; that is, it makes the ABNF less readable.

@martinthomson
Copy link
Contributor

Indirection means DRY, which edges out that sort of readability for me.

@LPardue
Copy link
Contributor

LPardue commented May 17, 2023

I find the ANF confusing anyway. The prose says parse it as Integer or Decimal, then If output_date is a Decimal, fail parsing. For a library that cares to differentiate between a parse failure due to bad characters vs bad types, then the ABNF would imply that you're not allowed to parse the Decimal type correctly?

@Jxck
Copy link
Author

Jxck commented May 18, 2023

@LPardue IIUC, that's why this draft made ABNF as Appendix.
That caused since Algorithm has "Parse Number" returns Integer | Decimal, but not in ABNF.
What should be intends is just allow sf-integer, and in serialization point of view, it works fine.

@mnot
Copy link
Member

mnot commented May 24, 2023

I don't have particularly strong feelings about this; does anyone else?

@LPardue
Copy link
Contributor

LPardue commented May 24, 2023

The ABNF already has layers upon layers of indirection. I think trying to avoid it for this field is inconsistent. So I'm sort of with MT that DRY wins out.

@mnot mnot closed this as completed in 5451804 Jun 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

5 participants