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

Support single-character units for parsing #60

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

bartoncasey
Copy link

Adds support for parsing single-character units (k, m, g, t, p).

@dougwilson
Copy link
Contributor

dougwilson commented Nov 15, 2021

Thank you for your pull request. Isn't this already possible like the following?

input = '45k'
console.log(bytes(`{input}b`))

Can you elaborate on the purpose of this feature and how it would be used, etc.?

@bartoncasey
Copy link
Author

Thank you for your pull request. Isn't this already possible like the following?

Sure, but then you're just putting extra parsing logic into your application code.

Can you elaborate on the purpose of this feature and how it would be used, etc.?

It just makes the parsing a little more flexible when working with input from different sources. The meaning of the single-character unit is unambiguous, so I don't see why bytes shouldn't accept it gracefully.

@dougwilson
Copy link
Contributor

dougwilson commented Nov 15, 2021

The meaning of the single-character unit is unambiguous, so I don't see why bytes shouldn't accept it gracefully.

How will it interact with the addition of kib, mib, and similar units (#26)?

@bartoncasey
Copy link
Author

How will it interact with the addition of kib, mib, and similar units?

I suspect the least surprising thing to do would be to map it to classic binary units.

That would be a breaking change due to the current units switching to metric anyway, so it would probably be part of a bigger refactoring and api change.

@dougwilson
Copy link
Contributor

Gotcha. So we should wait on this to land support for the classic binary units first, then add the single letter alias as a mapping to those? This is likely a breaking change already, as folks may be relying on the existing parsing to reject '100k', for example.

@bartoncasey
Copy link
Author

/shrug. I have no idea about the current roadmap. I just hacked this into my local copy as part of my current project, and thought it would be generally useful.

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

Successfully merging this pull request may close these issues.

2 participants