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

migrate: Handle single newlines in WordPress comments as line breaks #903

Merged
merged 1 commit into from
Jun 12, 2022
Merged

migrate: Handle single newlines in WordPress comments as line breaks #903

merged 1 commit into from
Jun 12, 2022

Conversation

projectgus
Copy link
Contributor

@projectgus projectgus commented Jun 6, 2022

WordPress renders a single newline in a comment as a
tag, but Isso renders a single newline in the comment as a single newline in the HTML. This is rendered the same as if it was a space, all text on one line.

To fix, detect single newlines when importing WordPress comments and convert to a line break in Markdown. Add a test for this also.

Example, this WordPress comment (as shown in CDATA of XML export):

First line of comment.
Second line of comment.

Renders in WordPress as:

First line of comment.
Second line of comment.

But renders in Isso after import as if it was:

First line of comment. Second line of comment.

After this commit is applied and comments re-imported, it renders as:

First line of comment.
Second line of comment.

Checklist

  • All new and existing tests are passing
  • (If adding features:) I have added tests to cover my changes
  • (N/A) (If docs changes needed:) I have updated the documentation accordingly.
  • (N/A but please tell me if you disagree) I have added an entry to CHANGES.rst because this is a user-facing change or an important bugfix
  • I have written proper commit message(s)

@projectgus projectgus changed the title migrate: Handle single newlines in WordPress comments as line breaks Draft: migrate: Handle single newlines in WordPress comments as line breaks Jun 6, 2022
@projectgus projectgus changed the title Draft: migrate: Handle single newlines in WordPress comments as line breaks migrate: Handle single newlines in WordPress comments as line breaks Jun 6, 2022
@projectgus
Copy link
Contributor Author

I haven't touched any javascript in the PR so I don't think the js integration test failures are related. I don't have access to retry the test run, though.

@ix5
Copy link
Member

ix5 commented Jun 8, 2022

Hi @projectgus, thank you for your contribution! Especially adding a new test case is really helpful.
The e2e tests seem to have been flaky, sorry for that. Re-running (and testing the same locally), I cannot reproduce what caused that issue.

Re: Inserting a <br> element: Since some people disallow raw HTML in comments, I'd rather you do this the "native" Markdown way: Inserting two spaces at the end of the line, which will also render as a line break.

Edit: Please also remove the windows linebreak (\n\r) in the XML file.

@projectgus projectgus changed the title migrate: Handle single newlines in WordPress comments as line breaks migrate: Handle single newlines in WordPress comments as Markdown double newlines Jun 8, 2022
@projectgus
Copy link
Contributor Author

@ix5 That's funny, original version of this patch replaced with \n\n but then I second-guessed myself and changed to <br>. It's \n\n again now!

Also fixed the linebreak, nice catch. I think this was copypasta from the WordPress XML export.

Thanks for all your work on Isso, it's very useful. :)

isso/migrate.py Outdated Show resolved Hide resolved
@ix5
Copy link
Member

ix5 commented Jun 11, 2022

Looks good, thanks for addressing the review comments.

Please also add a CHANGES entry, under "bugfixes" for the "next" version (i.e. not under 1.13.0.beta1), and if you like, add yourself to CONTRIBUTORS

WordPress renders a single newline in a comment as a <br> tag, but Isso renders a
single newline in the comment as a single newline in the HTML. This is rendered
the same as if it was a space, all text on one line.

To fix, detect single newlines when importing WordPress comments and convert
to a line break in Markdown. Add a test for this also.

Example, this WordPress comment (as shown in CDATA of XML export):

> First line of comment.
> Second line of comment.

Renders in WordPress as:

> First line of comment.
> <br>
> Second line of comment.

But renders in Isso after import as if it was:

> First line of comment. Second line of comment.

After this commit is applied and comments re-imported, it renders as:

> First line of comment.
> Second line of comment.
@projectgus
Copy link
Contributor Author

@ix5 Done, and rebased. Thanks!

@projectgus projectgus changed the title migrate: Handle single newlines in WordPress comments as Markdown double newlines migrate: Handle single newlines in WordPress comments as line breaks Jun 12, 2022
@ix5 ix5 merged commit 30ef218 into isso-comments:master Jun 12, 2022
@ix5
Copy link
Member

ix5 commented Jun 12, 2022

Merged. Thank you for your contribution and don't hesitate to submit further PRs!

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