wanasit
Add single-character en time units
Hi there and Merry Christmas! This change looks great, but it could cause false positives. Would you be willing to refactor this into a configuration option so that it's opt-in?
Hi, Merry Christmas also! I will take a look at that later.
And since French already parses 'h', want to do the same there?
Sounds great! I'd prefer we keep existing library behavior as-is, as to not break dependent applications (so let's not change the french locale).
The way it works now has not altered the existing behaviour as far as I can find. Test results are also the same as before (1 failed).
Before I start adding tests, I would like to know whether this is how you intended it to work?
One possible flaw is that this option is currently ignored in strict mode for the parsers that handled that option already.
Besides this, I also noticed it doesn't consistently* parse conjunctions, would that require a new Parser?
For example: parsing in 2 days and 5 hours results in 2 days. Whilst in 2 hours and 19 minutes parses 2 hours and 19 minutes.
Hello sorry for my slow reply. Overall, really good works!
I know this is opposite to @benaubin suggestion, but I'm actually thinking we could detect those single character behavior by default. If we used those patterns in relative parsing, e.g. "in 25m" example, I think it almost always mean the time. (@benaubin do you have any significant false positive examples that you have in mind?)
One possible flaw is that this option is currently ignored in strict mode for the parsers that handled that option already.
Yes. strict mode is very subjective. And that's also the reason I think for the default (or casual) option, we should also detect those abbreviation by default.
If we really want to introduce another options, I'm thinking the useShorts should be true by default. And maybe includeAbbreviations might be more precise configuration name.
Besides this, I also noticed it doesn't consistently* parse conjunctions, would that require a new Parser? For example: parsing in 2 days and 5 hours results in 2 days. Whilst in 2 hours and 19 minutes parses 2 hours and 19 minutes.
Yes. Let fix that separately.
I don't have a preference for any of the cases (with/without option & on/off by default), as for our use case anything would work.
Whether it would be a problem to enable abbreviations by default, I'm not sure. In an edge case, 'In 25m' could mean in 25 meters, like when talking about route planning (which isn't particularly a use case for Chrono). It'll depend on how missing out on 'in 25m' as time versus a misinterpretation of 'in 25m' as distance weighs up to each other.
And yeah, I didn't have a good name for it. If the option makes it, then includeAbbreviations sounds good.