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

feat: allow import of newer pocket data export files in csv format #1023

Merged
merged 5 commits into from
Dec 11, 2024

Conversation

melnary
Copy link
Contributor

@melnary melnary commented Dec 9, 2024

Recently Pocket changed the format of their data export function, switching from HTML files to a zipped folder containing CSV files, delivered in parts of up to 10000 bookmarks.
Other bookmark managers were also affected by this, as seen here: hoarder-app/hoarder#570

This patch allows the pocket sub-command to handle these files correctly, expanding it's functionality to import both HTML and CSV files, as I believe the ability to import older exports may still be useful. This is done by a simple file extension check, which I think will suffice for most users.

Note that the command still does not process the multiple part files explicitly, so the user has to import them one by one, which I don't think will be too much of a hassle for the user either way. :)

I would appreciate it if someone could test this patch on an older HTML file (or give me one so I could test it myself :3), as I was only able to test it with my own CSV export.

Copy link

codecov bot commented Dec 11, 2024

Codecov Report

Attention: Patch coverage is 0% with 103 lines in your changes missing coverage. Please review.

Project coverage is 34.28%. Comparing base (87bc7a8) to head (2e5290a).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
internal/cmd/pocket.go 0.00% 103 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1023      +/-   ##
==========================================
- Coverage   34.71%   34.28%   -0.44%     
==========================================
  Files          61       61              
  Lines        5326     5393      +67     
==========================================
  Hits         1849     1849              
- Misses       3253     3320      +67     
  Partials      224      224              

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

@fmartingr
Copy link
Member

Hey @melnary, thanks for the contribution! I'm not super famiiar with pocket and their exports so for me if you tested it it's good to go. Can you take a look at the CI errors and correct them though?

@melnary
Copy link
Contributor Author

melnary commented Dec 11, 2024

@fmartingr Hi! I was wondering where the CI was at :3
Do we have something to address coverage in the cmd package?

@fmartingr
Copy link
Member

Dont worry about coverage, the linter errors are here: https://github.com/go-shiori/shiori/actions/runs/12276136781/job/34257042686#step:3:1

@melnary
Copy link
Contributor Author

melnary commented Dec 11, 2024

@fmartingr Please approve the workflows again, my local linter is reporting no errors, should be good to go!

@melnary
Copy link
Contributor Author

melnary commented Dec 11, 2024

Ah and one more thing! I did manage to track down an older HTML export file, to make sure that I didn't end up breaking the HTML import feature, luckily it all still works smoothly!

@fmartingr
Copy link
Member

Ah and one more thing! I did manage to track down an older HTML export file, to make sure that I didn't end up breaking the HTML import feature, luckily it all still works smoothly!

That's awesome!

Copy link
Member

@fmartingr fmartingr left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution!

@fmartingr fmartingr merged commit c2821ff into go-shiori:master Dec 11, 2024
12 of 14 checks passed
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.

2 participants