[doc] Use "param" instead of "code" to refer to parameters #2

[asmaloney]

Also fixes the set_font_size description in Theme.xml.

One of several commits to fix all of the class XML docs (see #64134).

[asmaloney]

There seems to be a bug in make_rst.py.

In TileMap.xml, if we convert tile_data on line 28 from [code]tile_data[/code] to [param tile_data], we end up with a Tag depth mismatch error.

Edit: It is only when all three of the params in this description are converted that we get this error... if we don't convert one of them it works properly.

[YuriSizov]

@asmaloney Can it be because there are several cases of [code]coords[/coords] in there (and not just in that description)? 🙃

[asmaloney]

lol - I'm using a regex find, and didn't even see that. Cross-eyed looking at all these.

Seems like something make_rst.py could catch & report.

Can these tag be nested?

[asmaloney]

Aside (and maybe not the right place for this):

Would it be reasonable to add a check in the ci for docs-only changes like this, skip the builds, and only do the static check? Or is it possible that the results of changing the docs can break a build?

The way I do it in my own repos is look for a special code ("[skip ci]") in the commit message and skip running the CI build steps. In this case maybe "[static checks only]" or "[skip build]" would be useful.

This would save a lot of time working on the docs and a lot of extra building on the GitHub side. Save the environment!

[YuriSizov]

Seems like something make_rst.py could catch & report.

Yeah, no idea why it doesn't complain about the issue as is, but starts to complain when you make this edit there. I'll see if we can add a check for unrecognized tags to catch this kind of typos.

Can these tag be nested?

Well, some can, and there is a counter in the logic there to make sure we properly handle that. But I guess it doesn't work perfectly.

Would it be reasonable to add a check in the ci for docs-only changes like this, skip the builds, and only do the static check? Or is it possible that the results of changing the docs can break a build?

Yes, I think that would be reasonable for some of the edits. I believe we discussed this before, but I don't remember if there was a reason not to do this, or if we agreed that it was good but never implemented it. Feel free to bring this on Rocket.Chat, if you want and more importantly know how to do this.

Technically, changes to the XML can break something in the built-in documentation, but I don't think we test editor_help.cpp in any way in CI. So even if that happens, we'd be none the wiser.

[asmaloney]

I downloaded and signed up to Rocket.Chat, but it's really unusable for me. I can't use dark modes because I get "eyeball burn-in" and headaches. I haven't found a way to change to a light theme.

[YuriSizov]

Unfortunately, I think the theme is enforced by server on Rocket.Chat. If you use it in browser, you may be able to override it with some extensions that can enforce light mode.

---

I figured why the [/coords] thing wasn't caught. The parser allows any [tag]-like string inside of the code block or inline code tag. It is all parsed as code until the closing tag is found. As a result, here's how it looks in the output:

So technically, it just successfully parses this, as it doesn't see it as a closing tag, but finds one later. Not sure if there is a good way to improve the parser here.

[asmaloney]

When you enter a [code] tag, could you add a "tag stack" and push when you see [foo], pop when you see [/foo], error if you see [/bar] which doesn't match the top of the stack?

Or does it really need to allow this?:

[code] ... blah blah [/bar] blah blah ... [/code]

[akien-mga]

I downloaded and signed up to Rocket.Chat, but it's really unusable for me. I can't use dark modes because I get "eyeball burn-in" and headaches. I haven't found a way to change to a light theme.

You can toggle between dark mode and light mode normally. At least on the mobile app on Android it's configurable. Can't check desktop right now but there should be a icon for it (possibly themed like a sun or light bulb).

[asmaloney]

Thanks @akien-mga - I'm not seeing it anywhere in the downloaded one (or online, which I assume is the same codebase). Nothing in the settings and just have these in the upper-right (which look to be just server-related things):

I'll look around a bit more later, but unfortunately this is a showstopper for me.

[Calinou]

The custom CSS is hardcoded, so you can't switch back to the default (light) theme.

I would have preferred to use @media (prefers-color-scheme: dark) to enable the dark theme only when requested, but the Electron app doesn't have a way to toggle this CSS media query. This means Electron app users would be stuck with a light theme, which is a bad thing for most regulars.

I'll look around a bit more later, but unfortunately this is a showstopper for me.

Can't you increase the font size or reduce screen brightness?

[YuriSizov]

Or does it really need to allow this?:

[code] ... blah blah [/bar] blah blah ... [/code]

That's how it's written, but I'm not sure. I'll tally the cases of [something] inside of code blocks tomorrow and try to make the rules stricter to help catch such mistakes. I assume it's most likely needed for array literals, and maybe for some decorator syntax in C#?

[asmaloney]

Can't you increase the font size or reduce screen brightness?

It's actually not the brightness - it's the white text on black background. I get a "burn-in" effect with it (not sure how else to describe it).

It's fine - I'll skip it for now.

[YuriSizov]

I'll tally the cases of [something] inside of code blocks tomorrow and try to make the rules stricter to help catch such mistakes. I assume it's most likely needed for array literals, and maybe for some decorator syntax in C#?

So there is a good reason to support the syntax, so instead I added a warning, and not an error. I've also refactored the makre_rst.py's whole parser to be more organized, IMO, and fixed extra issues on top of that which I didn't even know about.

#64230

Since it fixes the [code]foo[/foo] cases, it would conflict with this PR. Depending on what we merge first, the other one would need a rebase. I'll try to review yours tomorrow (and any follow-up that you may make in the meantime).

PS. If you want to review my changes, btw, that'd be really great! Not many people maintain this part of our project, so extra eyes are appreciated.

[asmaloney]

👍 Great stuff.

Since it fixes the [code]foo[/foo] cases, it would conflict with this PR.

I fixed the two cases of [/coords] and forced-pushed the changes ☝️ - would it still conflict?

Either way rebasing is no problem!

[YuriSizov]

would it still conflict?

Yeah, if the other PR is merged first, then you're going to end up with a changeset that changes the same lines. And the same for me, if this one is merged first. It shouldn't be a big deal, I just wanted to warn ahead of time :)

[asmaloney]

Ah, yeah I see now - I'm looking at your changes.

[YuriSizov]

This is good. Added several adjustments. I'll push them myself in a minute. There seems to be a mixup in Theme.xml that I want to fix here, and I can't do it with just review comments.

[YuriSizov]

Thanks!