Page MenuHomePhabricator

Modify readme.md for gerrit markdown
Closed, ResolvedPublic

Description

The current readme.md in wvui assumes github markdown syntax (example: using the <details> tag. WVUI was moved from github to gerrit, and as such should use only gerrit supported markdown. Remove all <details> tags and any other syntax thats not supported.

Tagging Release-Engineering-Team for some input. Wondering if anyone knows a way to enable HTML blocks like github does? For example, the <details> tag mentioned above. I see the safehtml option here might suggest that it will disregard HTML blocks, but wondering if anyone has any more info!

Upstream ticket: https://github.com/google/gitiles/issues/219

Event Timeline

Krinkle subscribed.

Markdown does not have a standard syntax for collapsible elements, and the Google implementation of Markdown does not have an alternate way to do so either. The Google/Gitiles implementation does support various "GitHub Flavoured Markdown" syntaxes, but the <details>/<summary> blocks are afaik not considered GFM syntax, but rather part of the HTML elements that GitHub allow, thus they don't come along for the ride with that option it seems. We could ask upstream to add it, though.

Arbitrary HTML blocks won't be enabled for security reasons.

Note that one can preview what it would look like in Gerrit before merging by following the "gitiles" links within the interface, including e.g. when using the online commit editor to render an unsaved edit.

+1 to filing an issue with upstream. We have a very limited gitiles config in production currently, and mostly use the defaults; i.e., "githubflavored" markdown which has a pretty loose definition.

One similar thing might be the markdown.blocknote setting: https://gerrit.googlesource.com/gitiles/+/refs/heads/master/Documentation/markdown.md#notification_aside_promotion-blocks -- if it's useful to you all that seems innocuous to enable.

Cool thanks for the input guys! I've filed an issue upstream. I'm not sure about using blocknote, im giving it a try and it looks like github renders it as a <hr> element? So maybe its best to use something that would be consistent on both github and gerrit!

Aklapper updated the task description. (Show Details)
Aklapper moved this task from Backlog to Reported Upstream on the Upstream board.

Change 647753 had a related patch set uploaded (by Nikki Nikkhoui; owner: Nikki Nikkhoui):
[wvui@master] Make readme.md gitiles markdown compliant

https://gerrit.wikimedia.org/r/647753

Change 647753 merged by jenkins-bot:
[wvui@master] Make readme.md gitiles markdown compliant

https://gerrit.wikimedia.org/r/647753

@thcipriani what do you think about enabling markdown.namedanchors here. Found some other parts of our readme that weren't easily translatable, this one seems straightforward to enable!

Change 650598 had a related patch set uploaded (by Nikki Nikkhoui; owner: Nikki Nikkhoui):
[operations/puppet@production] Enable gitiles named anchor

https://gerrit.wikimedia.org/r/650598

Change 650598 merged by Dzahn:
[operations/puppet@production] Enable gitiles named anchor

https://gerrit.wikimedia.org/r/650598

@Dzahn do the changes take time to take affect or should it be immediate?

@nnikkhoui It should be immediate. Unless gerrit needs a restart, but I did not expect this to be the case for a gitiles config change. I might be wrong though? cc; @Paladox @thcipriani

Gerrit needs a restart I think for that to take effect.

@Dzahn Actually you could see if reloading the plugin works (which doesn't require downtime).

https://gerrit-review.googlesource.com/Documentation/cmd-plugin-reload.html

@Paladox Thanks for that, I tried but:

ssh -p 29418 dzahn@gerrit.wikimedia.org gerrit plugin reload
fatal: remote plugin administration is disabled

though I guess its disabled for a reason, so I think restarting gerrit is the only way.

though I guess its disabled for a reason, so I think restarting gerrit is the only way.

I'll try that this afternoon if no one beats me to it

Mentioned in SAL (#wikimedia-operations) [2020-12-22T19:32:15Z] <mutante> restarting gerrit to pick up config change in gitiles for T269300

@Dzahn I'm not seeing the changes BUT I realized this is likely my fault, I think markdown.safehtml also needs to be enabled for named anchors to be true.... -_-
Apologies, I will submit another patch to add that!

Change 651614 had a related patch set uploaded (by Nikki Nikkhoui; owner: Nikki Nikkhoui):
[operations/puppet@production] Enable safehtml in gitiles

https://gerrit.wikimedia.org/r/651614

@Dzahn I'm not seeing the changes BUT I realized this is likely my fault, I think markdown.safehtml also needs to be enabled for named anchors to be true.... -_-
Apologies, I will submit another patch to add that!

hrm, this should be true by default since it follows githubFlavor that is true by default.

@Dzahn I'm not seeing the changes BUT I realized this is likely my fault, I think markdown.safehtml also needs to be enabled for named anchors to be true.... -_-
Apologies, I will submit another patch to add that!

hrm, this should be true by default since it follows githubFlavor that is true by default.

On my local setup this seems to be the case. That is, explicitly setting safehtml to true is not needed.

I notice that namedanchors are working on our gitiles as well: for example: https://gerrit.wikimedia.org/g/wvui/+/refs/heads/master#i_o-performance-on-macos

@thcipriani okay so this was definitely a case of PEBCAK haha I was trying to reference those anchors elsewhere on the page and didn't see that working, but i was doing so incorrectly. I just uploaded a new patch aaaand i see it working now :) thanks for clearing that up

Change 651614 abandoned by Nikki Nikkhoui:
[operations/puppet@production] Enable safehtml in gitiles

Reason:
This is set to true by default

https://gerrit.wikimedia.org/r/651614

I just uploaded a new patch aaaand i see it working now :) thanks for clearing that up

\o/ great!

Thanks for the improvement of our gerrit's markdown config!

Change 651196 had a related patch set uploaded (by Nikki Nikkhoui; owner: Nikki Nikkhoui):
[wvui@master] Add explicit named anchor in readme

https://gerrit.wikimedia.org/r/651196

Change 651196 merged by jenkins-bot:
[wvui@master] Add explicit named anchor in readme

https://gerrit.wikimedia.org/r/651196

Volker_E triaged this task as Medium priority.Feb 1 2021, 4:45 PM
Volker_E removed a project: Patch-For-Review.
Volker_E moved this task from Backlog to Done on the WVUI board.