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

Removed HTML tags from Markdown trait export #19403

Merged
merged 1 commit into from
Sep 18, 2021

Conversation

Mailaender
Copy link
Member

This is a workaround for mkdocs/mkdocs#2400, but I thought it might actually be nicer as Markdown is easier for humans to read compared to hard-coded HTML tags.

@pchote
Copy link
Member

pchote commented May 9, 2021

Looks reasonable, but have you tested it works? I usually find it takes 5+ attempts to get tables to actually render whenever I try to make one.

The other doc generating commands should be changed to match.

@Mailaender
Copy link
Member Author

I tested with @retext-project and @mkdocs locally.

abcdefg30
abcdefg30 previously approved these changes May 18, 2021
Copy link
Member

@abcdefg30 abcdefg30 left a comment

Choose a reason for hiding this comment

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

Works nicely.

The other doc generating commands should be changed to match.

That is still open, isn't it?

@Mailaender
Copy link
Member Author

If you insist. As MarkDown forbidds linebreaks in tables I had to come up with something else for the Lua page.

@pchote
Copy link
Member

pchote commented May 18, 2021

What do you mean? Do you have a before and after image? Some quick searching suggests that <br> should work fine in tables...

@Mailaender
Copy link
Member Author

| Test

line break Test
Test without line break Test

@Mailaender
Copy link
Member Author

Mailaender commented May 18, 2021

I came up with forced tables for the Lua API to preserve your original layout although I think headerless tables were a really bad choice here. It really gave me a hard time here and it isn't very readable in .md viewers.

@pchote
Copy link
Member

pchote commented May 18, 2021

Test
line break
using <br />
Test

Copy link
Member

@abcdefg30 abcdefg30 left a comment

Choose a reason for hiding this comment

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

Github renders the triple * directly and not as bold text:
grafik

Using only **text** fixed the issue for me.

@Mailaender
Copy link
Member Author

Should be fixed now.

@abcdefg30
Copy link
Member

Unfortunately, there seems to be some kind of issue with line breaks(?):
grafik

@abcdefg30
Copy link
Member

I put the output file I got at https://gist.github.com/abcdefg30/6610bdfaa06e4c00dd209a71dd83aa0c, then you can hopefully easily see the spots with that issue. (Or is that a bug in the GitHub Markdown renderer?)

@Mailaender
Copy link
Member Author

A Markdown table is required to have a header, which is why @pchote original design is causing trouble here:

|---|---|
| int BuildTime(string type, string queue = nil) | Returns the build time (in ticks) of the requested unit type.
An optional second value can be used to exactly specify the producing queue type. |
| int Cost(string type) | |
| Actor Create(string type, bool addToWorld, LuaTable initTable) | Create a new actor. initTable specifies a list of key-value pairs that defines the initial parameters for the actor's traits. |
| int CruiseAltitude(string type) | Returns the cruise altitude of the requested unit type (zero if it is ground-based). |

won't work.

@Mailaender Mailaender force-pushed the docs-md-table branch 2 times, most recently from e9081a0 to d35dd03 Compare June 15, 2021 10:21
@Mailaender
Copy link
Member Author

I simply added headers with contents as GitHub flavored Markdown and many others don't support headerless tables. https://stackoverflow.com/questions/17536216/create-a-table-without-a-header-in-markdown

@pchote
Copy link
Member

pchote commented Jun 15, 2021

GitHub flavored markdown does support headerless table, I demonstrated this in the comment above: #19403 (comment).

Edit: I see, that was the header.

@matjaeck

This comment has been minimized.

@matjaeck
Copy link

matjaeck commented Jun 16, 2021

Since the previous comment has been marked off-topic:

An alternative way to highlight code in HTML-tables is:

<table>
<tr><th>Property</th><th>Default Value</th><th>Type</th><th>Description</th></tr>
<tr><td>ValidTypes</td><td></td><td>Set of String</td><td>Accepted <code>DeliversCash</code> types. Leave empty to accept all types. </td></tr>
<tr><td>ValidRelationships</td><td>Ally</td><td>PlayerRelationship</td><td>Player relationships the owner of the delivering actor needs. </td></tr>
<tr><td>Sounds</td><td></td><td>Collection of String</td><td>Play a randomly selected sound from this list when accepting cash. </td></tr>
</table>
<code>DeliversCash</code>

This is valid HTML, perhaps someone misread the comment…

Edit: Deleted some formatting changes and actual off-topic moderation-related content since i just wanted to drop this information here and not engage in discussions. Maybe it helps, maybe not.

@Mailaender
Copy link
Member Author

But then there is nothing Markdown left and both @github and @readthedocs take that as input. The HTML is compiled from it.

@pchote
Copy link
Member

pchote commented Jun 16, 2021

Why do we care about eliminating html from this? Surely the goal should be in having the best quality rendered output.

Copy link
Member

@abcdefg30 abcdefg30 left a comment

Choose a reason for hiding this comment

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

Lgtm once that is fixed, I don't think we need or should go back now.

abcdefg30
abcdefg30 previously approved these changes Jun 17, 2021
Copy link
Member

@pchote pchote left a comment

Choose a reason for hiding this comment

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

Looks fine aside from two points:

@Mailaender
Copy link
Member Author

Updated.

@Mailaender
Copy link
Member Author

Updated.

@pchote
Copy link
Member

pchote commented Jul 11, 2021

I'm not sure if the last push broke it or if I just didn't notice it before, but the output of the lua player properties/commands all end up on one line (the other two lua types are output correctly).

@Mailaender
Copy link
Member Author

Should be fixed now.

@pchote pchote merged commit eba266a into OpenRA:bleed Sep 18, 2021
@Mailaender Mailaender deleted the docs-md-table branch September 18, 2021 22:07
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.

None yet

4 participants