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

Making Tod::TimeOfDay.parse more flexible #70

Open
fig opened this issue May 5, 2020 · 1 comment · May be fixed by #71
Open

Making Tod::TimeOfDay.parse more flexible #70

fig opened this issue May 5, 2020 · 1 comment · May be fixed by #71

Comments

@fig
Copy link
Contributor

fig commented May 5, 2020

Proposal

Currently, the only separator permitted by the parser is a colon (:)

How do you feel about making the parser more flexible by allowing any non-digit character to be treated as a seperator?

Rationale

I work in the rail transport sector and railway timetables can contain a whole variety of separators, each with their own meaning:

  • 08.00 - Regular passenger train
  • 08+00 - Empty coaching stock
  • 08S00 - Stop for staff pick up / set down only
  • 08RM00 - Train will make a reversing move. (Depart in the same direction it arrived)

I'm sure there are other areas, including user input via html forms, where greater flexibilty might be beneficial.

Allowing any separator will make parsing any data more flexible and simpler.

Problems

The change could possibly break clients that rely on Tod parsability to validate data that must only use a colon as a seperator

@fig fig linked a pull request May 5, 2020 that will close this issue
@jackc
Copy link
Owner

jackc commented May 13, 2020

I think this might be a bit too flexible. If I read it correctly a string such as 2 times 11 is 22 would successfully parse into 02:11:22. I would expect an error and would consider that "successful" result surprising.

When this flexibility is needed I would suggest adding the behavior via sub-classing or monkey patching.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants