ably / specification

The Ably features spec for client library SDKs.
Apache License 2.0
0 stars 4 forks source link

Format IDL by presenting it within an HTML `pre` element #116

Closed QuintinWillison closed 1 year ago

QuintinWillison commented 1 year ago

We only had one location in the textile documents moved over from the original ably/docs home that this needed fixing in, being the IDL within our features spec.. There was no need for 'Block code' (code elements within outer pre element) so I've kept it simple and replaced with just Pre-formatted text.

When features.textile was rendered by the tooling in the docs repository then this backtick-fenced block was passing through this filter, which is very clever but completely over the top for what we need in this repository context. Let's, instead, just use basic textile functionality - which is what the change in this pull request does.

Closes #11.

QuintinWillison commented 1 year ago

Thanks, @owenpearson - while it's in rem rather than em (see tailwind docs), does https://github.com/ably/specification/pull/116/commits/0157482b344b2833ce4ad9e6c43218f68eb108a9 improve this for you?

owenpearson commented 1 year ago

@QuintinWillison I was actually thinking more about vertical margin, I think it looks quite compact with no spaces between the classes (although I think the horizontal margin is an improvement too). An alternative approach is to just put the whole IDL in one pre element since they preserve whitespace.

QuintinWillison commented 1 year ago

I hadn't noticed that my change had the effect of taking away space between the classes in the IDL, that's not nice. The textile processor is producing multiple pre elements, which I hadn't expected, nor had I checked for. I shall move this PR back to Draft while I play some more.

QuintinWillison commented 1 year ago

@owenpearson, between https://github.com/ably/specification/pull/116/commits/227bd5dc9664edf7591b788702531d6ce1d4379d and https://github.com/ably/specification/pull/116/commits/3f9b1a4c66bec678e7507e90f5681d4e77153ecf I've hopefully addressed your observation around spacing plus further improved the formatting.

QuintinWillison commented 1 year ago

ok, I've now made it look rubbish elsewhere - e.g. here. Am going to roll back to Draft again so I can fix that up. Sorry for the noise.

QuintinWillison commented 1 year ago

This isn't quite the small and quick pull request I had hoped it would be this morning, but on the flip side I think we're tidying up presentation across the board so it's a good improvement. I've removed other 'Block code' instances in https://github.com/ably/specification/pull/116/commits/b552ae1ddb8d0744f06c23d885a1d52699125989 - looking at how GitHub markup code blocks, they only use pre elements too, so this feels like the appropriate approach going forwards (also reading MDN docs on pre vs code).