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 additional numeric literals: octal, binary, hexadecimal and numeric separators #177

Closed
wojciechczerniak opened this issue Feb 17, 2020 · 14 comments
Labels
API Public methods and properties Feature Something we can add later on without introducing a breaking change

Comments

@wojciechczerniak
Copy link
Contributor

Description

Since #122 we support literal JS numeric formats but limited to integers and floats.

There are more numeric literals in JS: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Lexical_grammar#Numeric_literals

They should be handled by #176 as an unsupported input ATM so this can be treated as a feature request for the future.

Links

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Lexical_grammar#Numeric_literals

@wojciechczerniak wojciechczerniak added API Public methods and properties Feature Something we can add later on without introducing a breaking change labels Feb 17, 2020
@izulin izulin self-assigned this Feb 17, 2020
@wojciechczerniak wojciechczerniak added this to the Februrary 2020 milestone Feb 18, 2020
@izulin
Copy link
Collaborator

izulin commented Feb 25, 2020

is this issue about:

parsing extra formats, so we could write

HyperFormula.buildFromArray([
      ['0xFFF', '0b10101'],
    ]);

or

supporting extra formats as a raw type:

HyperFormula.buildFromArray([
      [0xFFF, 0b10101],
    ]);

?

@wojciechczerniak
Copy link
Contributor Author

The second one:

HyperFormula.buildFromArray([
      [0xFFF, 0b10101],
    ]);

This is just a text:

HyperFormula.buildFromArray([
      ['0xFFF', '0b10101'],
    ]);

@izulin
Copy link
Collaborator

izulin commented Feb 25, 2020

then it is already automatically supported (its on the JS side, not ours)

@wojciechczerniak
Copy link
Contributor Author

Awesome!

Can we add a test then? Binary, Octal, Hex, BigInt and Numeric Separator.

@izulin
Copy link
Collaborator

izulin commented Feb 25, 2020

do we want BigInt support? this is potentially a large rework (coercions, math functions, etc)?

@izulin izulin mentioned this issue Feb 25, 2020
7 tasks
@wojciechczerniak
Copy link
Contributor Author

No. We don't want it to throw an error either

@izulin
Copy link
Collaborator

izulin commented Feb 25, 2020

what do you mean? what is the desired behaviour then?

@wojciechczerniak
Copy link
Contributor Author

A message that it is not supported #195

@izulin
Copy link
Collaborator

izulin commented Feb 25, 2020

Like a console.log() warning?

should we also try to cast it to float?

@wojciechczerniak
Copy link
Contributor Author

Like a console.log() warning?

I was thinking about throwing the Unable to parse value: [] we already have. Maybe we should add type to those messages, though? BigInt may look like an Int and therefore might be confusing to the developer.

should we also try to cast it to float?

IMO No. No magic. Either we support it or not.

@izulin izulin mentioned this issue Feb 25, 2020
7 tasks
@izulin
Copy link
Collaborator

izulin commented Feb 25, 2020

Like a console.log() warning?

I was thinking about throwing the Unable to parse value: [] we already have. Maybe we should add type to those messages, though? BigInt may look like an Int and therefore might be confusing to the developer.

should we also try to cast it to float?

IMO No. No magic. Either we support it or not.

Since we merged other PR, we now have toolset for such error messages. Added.

@wojciechczerniak
Copy link
Contributor Author

Done in #204 and #208

@wojciechczerniak
Copy link
Contributor Author

@izulin Since BigInt was added in TypeScript 3.2 we need to bump the dependencies because it will not build the HF with lower version

"typescript": "^3.0.3",

@izulin izulin mentioned this issue Feb 26, 2020
7 tasks
@wojciechczerniak
Copy link
Contributor Author

Typescript requirements bumped in #211 Thx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Public methods and properties Feature Something we can add later on without introducing a breaking change
Projects
None yet
Development

No branches or pull requests

2 participants