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

Provides an extended bookmark export #1470

Conversation

Riduidel
Copy link

@Riduidel Riduidel commented May 19, 2020

THis provides a richer export, incompatible with old browsers, but allowing nicer integration mechanisms into other systems.
I typically gonna use that to generate a lifestream including my Shaarli content.

Fixes #1450

THis provides a richer export, incompatible with old browsers, but allowing nicer integration mechanisms into other systems.
I typically gonna use that to generate a lifestream including my Shaarli content.
Comment on lines 63 to 64
<span class="label-name">{'Extend import to include thumbnail and permalink|t}</span><br>
<span class="label-desc">{'Provide a richer export, unfortunatly incompatible with browsers'|t}</span>
Copy link
Member

Choose a reason for hiding this comment

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

The French translation is officially supported, so this PR should include the updated translation. I assume you're French, but let me know if you need help.

Copy link
Member

Choose a reason for hiding this comment

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

Also, why is it incompatible with browsers? Isn't only included the description?
Does it work as export -> import into Shaarli?

Copy link
Author

Choose a reason for hiding this comment

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

It' may incompatible with browsers in the sense that the export format reverse engineered by @sebsauvage doesn't include any supplmenetary HTML code. By adding code here, I take the risk of not maintaining that compatibility.
And frankly, I have absolutly not the courage to test it on all browsers, so this message is more a caution message than anything else.

Copy link
Author

Choose a reason for hiding this comment

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

The French translation is officially supported, so this PR should include the updated translation. I assume you're French, but let me know if you need help.

Yup, I would gladly get some help ... do you have any tutorial on PHP gettext usage for Windows users ?

Copy link
Member

Choose a reason for hiding this comment

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

any tutorial on PHP gettext usage for Windows users ?

Please have a look at https://shaarli.readthedocs.io/en/doc-rework-setup/dev/Translations/ and tell us if any information is missing (part of the documentation refactoring which is not merged yet)

Copy link
Member

Choose a reason for hiding this comment

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

test it on all browsers

We should probably test it in Firefox and Chrome/Chromium on Linux/Windows so that we can make the warning more accurate. I can take care of it (it will be faster if you can send an example export .html directly here).

Does it work as export -> import into Shaarli?

Important part. Maybe add some test cases to https://github.com/shaarli/Shaarli/blob/master/tests/netscape/BookmarkImportTest.php ?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, it works exactly like the standard export feature of Shaarli.

@ArthurHoaro ArthurHoaro changed the title Fixes #1450 Provides an extended bookmark export May 22, 2020
Riduidel and others added 2 commits May 25, 2020 09:09
According to ArthurHoaro, which knows better PHP than I do, this syntax is cooler ... and it is !

Co-authored-by: ArthurHoaro <[email protected]>
Copy link
Member

@ArthurHoaro ArthurHoaro left a comment

Choose a reason for hiding this comment

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

I checkout your branch to run some tests and encountered multiple template errors. Please make sure that the export + reimport back into Shaarli works properly.

Riduidel and others added 2 commits June 5, 2020 15:22
Forgot one quote

Co-authored-by: ArthurHoaro <[email protected]>
A small wording error

Co-authored-by: ArthurHoaro <[email protected]>
@Riduidel
Copy link
Author

Riduidel commented Jun 5, 2020

I checkout your branch to run some tests and encountered multiple template errors. Please make sure that the export + reimport back into Shaarli works properly.

OK, so, to test, I create the Docker image, run it to create a link, export that link, delete the data dir, import that link. Is this test procedure correct to you ?

Besides, while testing that, I had to change the python:3-alpine to python:3 (mkdocs has a dependency which requires compilation and gcc, which the python:3-alpine doesn't include)

@Riduidel
Copy link
Author

Riduidel commented Jun 5, 2020

OK, I've tested and fixed the export template.

ArthurHoaro added a commit to shaarli/netscape-bookmark-parser that referenced this pull request Jun 6, 2020
This change allows to have a <a> tag in the bookmark description.

See: shaarli/Shaarli#1470
@ArthurHoaro
Copy link
Member

I've run some tests, it looks fine. I ran into a few issues that were actually due to the netscape bookmark parser dependency. I have merged a fix in this project a released a new version. Can you run composer update and commit the composer.lock in your PR?

I tried to import it in Firefox, it's working but it also import all permalinks as distinct entries.

@Riduidel
Copy link
Author

Riduidel commented Jun 6, 2020 via email

@ArthurHoaro ArthurHoaro added this to the 0.12.1 milestone Sep 12, 2020
@ArthurHoaro ArthurHoaro modified the milestones: 0.12.1, 0.12.2 Nov 3, 2020
@nodiscc
Copy link
Member

nodiscc commented Jan 14, 2022

This needs to be rebased on master

@nodiscc nodiscc self-assigned this Aug 10, 2022
@nodiscc
Copy link
Member

nodiscc commented Sep 4, 2022

Since this patch is old, and other "rich" export solutions are being discussed in #1450 (probably not using the Netscape HTML format), I suggest closing this and starting over once we know exactly what's expected (a full JSON export, without the need for a third-party API client, would do the trick in my opinion). What do you think?

@nodiscc nodiscc removed their assignment Sep 4, 2022
@nodiscc nodiscc modified the milestones: 0.12.2, backlog to the future Sep 4, 2022
@nodiscc
Copy link
Member

nodiscc commented Oct 26, 2023

Closing in favor of #1450

@nodiscc nodiscc closed this Oct 26, 2023
@Riduidel Riduidel deleted the features/thumbnail_and_permalink_in_export branch November 29, 2023 07:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How can I provide an extended export ?
3 participants