use default commodity and commodity directives when inferring decimal point/digit group separator #635
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ony
force-pushed
the
bug/487-digits-groups-separator-ambiguity
branch
2 times, most recently
from
61265d2
to
0a73008
Compare
ony
force-pushed
the
bug/487-digits-groups-separator-ambiguity
branch
3 times, most recently
from
fe9df19
to
4297b95
Compare
ony
force-pushed
the
bug/487-digits-groups-separator-ambiguity
branch
from
4297b95
to
75decad
Compare
ony
force-pushed
the
bug/487-digits-groups-separator-ambiguity
branch
from
7d83698
to
e42fbd3
Compare
ony
added
needs:rebase
|
I trust your judgement that this works better. We should have haddocks for the two get*Style functions, and an update to the manual explaining how things work now. |
simonmichael
added
A-WISH
hledger-lib/Hledger/Read/Common.hs
Outdated
| @@ -145,6 +146,12 @@ setDefaultCommodityAndStyle cs = modify' (\j -> j{jparsedefaultcommodity=Just cs | |||
| getDefaultCommodityAndStyle :: JournalParser m (Maybe (CommoditySymbol,AmountStyle)) | |||
| getDefaultCommodityAndStyle = jparsedefaultcommodity `fmap` get | |||
|
|
|||
| getDefaultDecimalHint :: JournalParser m (Maybe Char) | |||
There was a problem hiding this comment.
Let's add short haddocks explaining what these do.
|
@simonmichael , as you probably noticed expecting that If you think we should have a separate directive for that (probably still currency-specific) it should be easy to add that. |
- Hunt down adjacent punctuations with altering char. - Add some tests dedicated to parsing amounts.
Make use of commodity format directive as a hint for parsing amount. Kinda resolves simonmichael#487
If appropriate commodity directive is missing fallback to default commodity directive to get number representation style.
Use whole AmountStyle in process of resolving decimal/groups separator ambiguity. Resolve simonmichael#399
ony
force-pushed
the
bug/487-digits-groups-separator-ambiguity
branch
from
e42fbd3
to
6f25bd1
Compare
|
The doc looks reasonable to me. Merging, thank you! |
simonmichael
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Trust latest
commodityorD(default commodity) directives in case of ambiguity in parsing number.This kinda resolves #399 and its duplicate #487. We still may get issues caused by misinterpreting numbers
1,000and1.0both as1if nocommoditydirective or default commodityDdirective were provided in same journal file earlier.