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

Implement support for !Type:Prices QIF records. #1744

Merged
merged 7 commits into from
Sep 10, 2023

Conversation

jwhite66
Copy link
Contributor

No description provided.

Copy link
Contributor

@christopherlam christopherlam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just an o(n^2) fix.

gnucash/import-export/qif-imp/qif-to-gnc.scm Outdated Show resolved Hide resolved
@christopherlam
Copy link
Contributor

LGTM...

Comment on lines 1394 to 1397
(set-tm:hour tm 11)
(set-tm:min tm 0)
(set-tm:sec tm 0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use 10:59:00Z for neutral time because 11:00:00Z is 00:00:00 the next day in New Zealand summer time.

It is 10x faster against a large dataset.

Insight from Christopher Lam.
Found a live Quicken record with an empty price.
The price import will now throw warnings if there are
invalid prices.  The previous logic would 'auto-next' in this
case, only holding if the parse-file phase threw warnings.

This change insures that these warnings are seen.
Including a few deliberate errors.
Also use a more standard numberic parser instead of an ad-hoc
string parser.

Code written by Christopher Lam.
The expected results changed with commit 80f7e60f49.
At the same time, improve readability of test results by
assigning a tag and ordering expected vs actual correctly.
@code-gnucash-org code-gnucash-org merged commit ab63595 into Gnucash:stable Sep 10, 2023
3 checks passed
@jralls
Copy link
Member

jralls commented Sep 10, 2023

I took care of neutral time and merged. Thanks!

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 this pull request may close these issues.

4 participants