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

Drop xmlparsedata and replace it with getParseData #659

Merged
merged 35 commits into from
Feb 23, 2024

Conversation

Ellpeck
Copy link
Member

@Ellpeck Ellpeck commented Feb 15, 2024

No description provided.

@Ellpeck Ellpeck linked an issue Feb 15, 2024 that may be closed by this pull request
Copy link

codecov bot commented Feb 15, 2024

Codecov Report

Attention: Patch coverage is 89.83957% with 19 lines in your changes are missing coverage. Please review.

Project coverage is 71.37%. Comparing base (f07212f) to head (b950109).

Files Patch % Lines
src/cli/repl/commands/parse.ts 46.15% 7 Missing ⚠️
src/r-bridge/lang-4.x/ast/parser/csv/parser.ts 91.89% 2 Missing and 1 partial ⚠️
src/cli/repl/server/connection.ts 0.00% 0 Missing and 2 partials ⚠️
src/r-bridge/retriever.ts 85.71% 0 Missing and 2 partials ⚠️
src/core/print/parse-printer.ts 83.33% 1 Missing ⚠️
.../r-bridge/lang-4.x/ast/parser/xml/internal/meta.ts 93.75% 0 Missing and 1 partial ⚠️
...lang-4.x/ast/parser/xml/internal/structure/root.ts 75.00% 0 Missing and 1 partial ⚠️
src/r-bridge/lang-4.x/values.ts 66.66% 0 Missing and 1 partial ⚠️
src/statistics/statistics.ts 83.33% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #659      +/-   ##
==========================================
- Coverage   71.58%   71.37%   -0.22%     
==========================================
  Files         219      218       -1     
  Lines        7145     7082      -63     
  Branches     1103     1092      -11     
==========================================
- Hits         5115     5055      -60     
+ Misses       1739     1737       -2     
+ Partials      291      290       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Ellpeck
Copy link
Member Author

Ellpeck commented Feb 15, 2024

Some notes from @EagleoutIce:

  • We want to rip out the old xmlparsedata parsing completely as part of this PR, this is meant to be a full replacement
  • We also want to stop using xml altogether and parse the table csv straight into a proper tree

@Ellpeck
Copy link
Member Author

Ellpeck commented Feb 22, 2024

oh fucking hell

@Ellpeck Ellpeck requested a review from EagleoutIce February 22, 2024 15:57
@Ellpeck
Copy link
Member Author

Ellpeck commented Feb 23, 2024

This is pretty much review-ready, except for the type safety of XmlBasedJson which is quite a lot of work

@Ellpeck Ellpeck marked this pull request as ready for review February 23, 2024 13:39
@EagleoutIce EagleoutIce merged commit 5617885 into main Feb 23, 2024
20 of 21 checks passed
@EagleoutIce EagleoutIce deleted the 653-drop-xmlparsedata branch February 23, 2024 14:06
EagleoutIce added a commit that referenced this pull request Feb 24, 2024
* refactor: construct children collection once

* lint-fix: specify return type of isRoot directly

* refactor: further performance & style improvements

* refactor: helpless panic

* refactor: another iteration of urgh

* Revert "refactor: another iteration of urgh"

This reverts commit e0bf428.

* refactor: swapping tries for gods sake

* Revert "refactor: swapping tries for gods sake"

This reverts commit b77cebe.

* Reapply "refactor: another iteration of urgh"

This reverts commit 63c19a2.

* refactor(wip): interim mental breakdown

* refactor: at least... happy tests?

* refactor: lock jsonlite options

* refactor(cfg-test): remove fake test

* refactor: try manual conversion, is worse

* Revert "refactor: try manual conversion, is worse"

This reverts commit 578c64e.

* refactor: simplify json conversion

* refactor: several minor performance tunes :3

* refactor: ensure the executor dies directly (and gracefully)

* refactor: dynamic `flowr_get` compile

* refactor: fine-tune cmp retrieval

* refactor: reduce size of json produce, defer obj map to flowR

* doc(test): remove dead jsdoc comment

* refactor: clean up tmp logfiles

* lint-fix: handle minor linter problems

* refactor: wordly clean-up

* feat: jsonlite begone!

* refactor: further improve print-out-performance

* refactor: always compile seems to be faster!

* refactor: use build for benchmark run

* refactor: switch back to `sort`

* feat(tests): cover for optional xmlparsedata availability

---------

Co-authored-by: Florian Sihler <[email protected]>
@EagleoutIce
Copy link
Member

This pull request is included in v1.4.2 (see Release v1.4.2 (Dropping xmlparsedata, Benchmark Re-Runs, and Repl Fixes)).

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.

Drop xmlparsedata and replace it with getParseData
2 participants