Skip to content

Additional Fluxbb tests and fixes#1

Closed
Firefishy wants to merge 12 commits into
harry-wood:fluxbb-tests-and-fixesfrom
Firefishy:fluxbb-tests-and-fixes
Closed

Additional Fluxbb tests and fixes#1
Firefishy wants to merge 12 commits into
harry-wood:fluxbb-tests-and-fixesfrom
Firefishy:fluxbb-tests-and-fixes

Conversation

@Firefishy

Copy link
Copy Markdown
  • Add [code] to markdown support
  • Use more standard <s> instead of github's ~~
  • Fix newline requirement for quote

harry-wood and others added 9 commits January 9, 2023 12:28
Allow a bbcode tag config to force all newlines into <br> html tags within its children_html. This is useful for outputting markdown headings as follows. Headings need to be all on one line even if the source bbcode allows newline chars.

`## My heading<br>continued on the next line.`

Outputting headings? This is something I need to be able to do as a custom "additional tag" at least.
Add an example tag config which makes use of the new 'to_br' feature. This is how we would use it for FluxBB (but iobviously doesn't effect default behaviour)
Treat tags as text inside code blocks
@Firefishy

Firefishy commented Jan 31, 2023

Copy link
Copy Markdown
Author

@harry-wood Any chance you could merge this PR into your branch? Maybe squash it.

@harry-wood

Copy link
Copy Markdown
Owner

I was very puzzled about why the ~~ strikethrough syntax would give issues (bear in mind that I tested this stuff on https://community.openstreetmap.org and https://meta.discourse.org/ and importing into a discourse docker image, so I know that ~~ works!) but I think I've got to the bottom of it

I see where someone was pointing out that it didn't work. https://community.openstreetmap.org/t/migrating-content-from-old-forums/446/152 . Their example coming from this original bbcode involving [s] within a [list]. I believe strikethrough must have worked fine everywhere else, but not within a list, and ... bad news... we have a [list] problem that goes beyond strikethrough.

I took the decision to do [list][*] syntax conversion to into html <ul><li> rather than trying to do markdown list syntax. I thought it would be more trouble free, given how awkward markdown lists are with their indentation. Trying to get the indentation right especially for nested lists... that's nasty. But I've just realised...

When we give discourse an <li> tag with content in it, all syntax support is disabled on that first line. Try pasting this into a discourse:

<ul>
<li>**bold?** *italic* ~~strikethrough~~  [url]http://google.com[/url]</li>
</ul>

But perhaps we can still avoid markdown list indentation hell. Try this.

<ul>
<li>

**bold?** *italic* ~~strikethrough~~  [url]http://google.com[/url]</li>
</ul>

Not pretty, but this works! So maybe we can just bung in some extra newlines after every <li>. The joys of discourse syntax parsing quirks.

So I guess I'll add that into the list handling logic, which is over in the discourse PR (regex preprocessing logic. Not in this gem)

Here we need to take out changes to do with strikethrough. Unless I'm mistaken, that was fine as I had it.

@Firefishy

Copy link
Copy Markdown
Author

~~ was not supported in discourse when I checked. See: https://meta.discourse.org/t/please-add-strikethrough-and-underline-support-in-markdown/22843

@Firefishy

Copy link
Copy Markdown
Author

I see where someone was pointing out that it didn't work. https://community.openstreetmap.org/t/migrating-content-from-old-forums/446/152 . Their example coming from this original bbcode involving [s] within a [list]. I believe strikethrough must have worked fine everywhere else, but not within a list, and ... bad news... we have a [list] problem that goes beyond strikethrough.

This was fixed. The older posts was where I was still testing multiple different versions of the importer.
There are some list item discourse specific fixes here: https://github.com/Firefishy/discourse/commits/osm-test-import

Please focus just on this PR.

@Firefishy

Copy link
Copy Markdown
Author

There have been no issues raised about the formatting on the test import site for the last few weeks despite me asking. In my view the fluxbb bbcode -> discourse markdown is now "good enough".

Please only look at comments AFTER the last import: https://community.openstreetmap.org/t/migrating-content-from-old-forums/446/164?u=firefishy

@harry-wood harry-wood force-pushed the fluxbb-tests-and-fixes branch from 9ae7307 to 6d7c8c8 Compare February 19, 2023 02:54
@harry-wood

Copy link
Copy Markdown
Owner

I didn't merge this directly, but I've just finished picking through it. We can close it. I've cherry-picked / taken inspiration from here into my PR: nlalonde#6

I didn't bring in the <s> change because I've done fix to the <li> issue over in the discourse PR discourse/discourse@5e86c43 ...which I believe is the only problem there.

@harry-wood harry-wood closed this Feb 19, 2023
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.

3 participants