Make pragmas look clickable in docs #8176

Quelklef · 2018-07-01T23:09:02Z

Hopefully I found the right file!

In documentation, the clickable {..} pragmas currently don't look clickable. They look like normal code and, given that it's not unreasonable to think that {..} is valid (if superfluous) code, that's what some users might think. I sure did.

Araq · 2018-07-02T07:09:28Z

Sorry, can you give us some nice before-after screenshots? :-)

Quelklef · 2018-07-02T12:04:33Z

It's nothing too wild...

Quelklef · 2018-07-02T12:08:27Z

A small alternative is to have a line between the begin and end pragma tags:

But it doesn't look as clean when the pragmas are shown. I could try to make the open pragma also have a border when the pragmas are shown, but I'm not sure how the clicking quite works yet.

dom96 · 2018-07-02T12:58:17Z

Wouldn't it make more sense to add a dotted line beneath these?

Quelklef · 2018-07-02T19:08:19Z

That could also work:

This is subtler but emphasises that the clickable pragmas are still part of the code.

Araq · 2018-07-03T10:14:06Z

The line between the dots is better, but still too subtle.

dom96 · 2018-07-03T11:54:14Z

Maybe something like this?

Alternatives:

Quelklef · 2018-07-03T12:05:13Z

Or perhaps something like:

It's terser, but looks a little strange with ... between two .s.

A possible resolution is (with the open graphics unchanged):

but then closed pragmas pragmas aren't {. and .}...

Araq · 2018-07-03T12:27:42Z

A possible resolution is (with the open graphics unchanged):

Yeah, I like this solution best.

dom96 · 2018-07-03T12:32:33Z

Yeah, I like this solution best.

Same.

Quelklef · 2018-07-03T12:55:11Z

If we're going with this, I think it's important to address one thing:

Clicking between the braces to reveal the pragmas signifies (to me at least) that one should click between the braces again to hide the pragmas. So the shown content should be clickable rather than the braces.

It would be a little weird, though, and frankly I don’t think doing it this way is a great idea. The content shown is conceptually that--content, not navigation. However, I'm not sure a great alternative. Show content on hover-on and hide it in hover-off would work, but that's also a bit janky.

Araq · 2018-07-03T12:59:08Z

If we're going with this, I think it's important to address one thing:

Clicking between the braces to reveal the pragmas signifies (to me at least) that one should click between the braces again to hide the pragmas. So the shown content should be clickable rather than the braces.

Let's just use {.+.} then and expanded {.- noSideEffect, more, stuff.} or something along these lines to give us enough visual clues.

dom96 · 2018-07-03T13:20:50Z

Nah, there is little need to hide the pragma after it has been shown. I say just keep the pragmas there with no option to hide them.

Araq · 2018-07-03T14:15:09Z

Or that, ok.

Quelklef · 2018-07-03T16:30:41Z

OK to change docgen.nim for this, or should be pure JS/CSS?

dom96 · 2018-07-03T16:49:56Z

We don't use JS when we can help it.

Quelklef · 2018-07-03T21:28:36Z

Unfortunately, because CSS is nondeterministic, I couldn't style it quite as nicely as I'd have liked, but here is the current state:

kaushalmodi · 2018-07-03T21:35:31Z

config/nimdoc.cfg

@@ -1330,15 +1330,23 @@ dt pre > span.Operator ~ span.Identifier {
  background-repeat: no-repeat;
  background-image: url("data:image/x-icon;base64,AAABAAEAEBAAAAEAIABoBAAAFgAAACgAAAAQAAAAIAAAAAEAIAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAD///8A////AP///wD///8A////AP///wD///8A////AP///wD///8A////AP///wD///8A////AP///wD///8A////AP///wD///8A////AP///wD///8A////AAAAAAUAAAAF////AP///wD///8A////AP///wD///8A////AP///wD///8A////AAAAAAIAAABbAAAAlQAAAKIAAACbAAAAmwAAAKIAAACVAAAAWwAAAAL///8A////AP///wD///8A////AAAAABQAAADAAAAAYwAAAA3///8A////AP///wD///8AAAAADQAAAGMAAADAAAAAFP///wD///8A////AP///wAAAACdAAAAOv///wD///8A////AP///wD///8A////AP///wD///8AAAAAOgAAAJ3///8A////AP///wAAAAAnAAAAcP///wAAAAAoAAAASv///wD///8A////AP///wAAAABKAAAAKP///wAAAABwAAAAJ////wD///8AAAAAgQAAABwAAACIAAAAkAAAAJMAAACtAAAAFQAAABUAAACtAAAAkwAAAJAAAACIAAAAHAAAAIH///8A////AAAAAKQAAACrAAAAaP///wD///8AAAAARQAAANIAAADSAAAARf///wD///8AAAAAaAAAAKsAAACk////AAAAADMAAACcAAAAnQAAABj///8A////AP///wAAAAAYAAAAGP///wD///8A////AAAAABgAAACdAAAAnAAAADMAAAB1AAAAwwAAAP8AAADpAAAAsQAAAE4AAAAb////AP///wAAAAAbAAAATgAAALEAAADpAAAA/wAAAMMAAAB1AAAAtwAAAOkAAAD/AAAA/wAAAP8AAADvAAAA3gAAAN4AAADeAAAA3gAAAO8AAAD/AAAA/wAAAP8AAADpAAAAtwAAAGUAAAA/AAAA3wAAAP8AAAD/AAAA/wAAAP8AAAD/AAAA/wAAAP8AAAD/AAAA/wAAAP8AAADfAAAAPwAAAGX///8A////AAAAAEgAAADtAAAAvwAAAL0AAADGAAAA7wAAAO8AAADGAAAAvQAAAL8AAADtAAAASP///wD///8A////AP///wD///8AAAAAO////wD///8A////AAAAAIcAAACH////AP///wD///8AAAAAO////wD///8A////AP///wD///8A////AP///wD///8A////AP///wD///8A////AP///wD///8A////AP///wD///8A////AP///wD///8A////AP///wD///8A////AP///wD///8A////AP///wD///8A////AP///wD///8A////AP///wD///8A//8AAP//AAD4HwAA7/cAAN/7AAD//wAAoYUAAJ55AACf+QAAh+EAAAAAAADAAwAA4AcAAP5/AAD//wAA//8AAA==");
  margin-bottom: -5px; }
+<<<<<<< HEAD


You committed some merge conflicts by mistake.

Ah, dammit. My bad. Will fix now.

kaushalmodi · 2018-07-03T21:36:07Z

config/nimdoc.cfg

 }
+=======
+>>>>>>> Visual cues at hidden pragmas in docs.


See those "<<<<<<<" and ">>>>>>>".

dom96 · 2018-07-03T21:56:02Z

compiler/docgen.nim

+    ## Remove leading whitespace and trailing newlines from every line
+    result = ""
+    for line in html.split("\n"):
+        for i, c in line:


Two space indent

Use splitLines and strip.

Araq · 2018-07-04T08:34:33Z

compiler/docgen.nim

@@ -222,6 +222,10 @@ proc getPlainDocstring(n: PNode): string =
      result = getPlainDocstring(n.sons[i])
      if result.len > 0: return

+func htmlCollapse(html: string): string =


Don't use func, it's too new. Also it's used nowhere else in docgen.nim.

Araq · 2018-07-04T08:35:45Z

compiler/docgen.nim

@@ -222,6 +222,10 @@ proc getPlainDocstring(n: PNode): string =
      result = getPlainDocstring(n.sons[i])
      if result.len > 0: return

+func htmlCollapse(html: string): string =
+  ## Strip whitespace and remove trailing newlines from every line
+  return html.splitLines().mapIt(strip(it)).join("")


Why is this required anyway?

This implementation has terrible performance. ;-)

I wrote it so I could format the HTML later in the code. Since that HTML ends up inside of a <pre>, it has to be collapsed.

The poor performance is from the string concatenation, yes? I should be using result.add?

Actually, a quick benchmark on my machine shows a ~1.6x speedup using result.add, except when in -d:release, in which case it's about the same or slower! So I'm not totally sure what optimizations you're looking for?

Quelklef · 2018-07-16T19:41:08Z

Ah, some styling that I thought was fixed is acting up again. Don't merge yet.

Quelklef · 2018-07-17T06:22:32Z

It should be good now. Unfortunately, I only have Chrome on my dev laptop so I have only tested on Chrome. I figure the changes are simple enough that it shouldn't be a problem. I can rig something up to do cross-browser testing if you'd like, though.

Quelklef · 2018-07-17T06:42:21Z

Update: apparently I also have Firefox 58.0 installed. It looks fine.

kaushalmodi reviewed Jul 3, 2018

View reviewed changes

config/nimdoc.cfg Outdated

}

=======

>>>>>>> Visual cues at hidden pragmas in docs.

Copy link

Contributor

kaushalmodi Jul 3, 2018

Choose a reason for hiding this comment

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

See those "<<<<<<<" and ">>>>>>>".

Visual cues for hidden pragmas in docs.

bc612f2

dom96 requested changes Jul 3, 2018

View reviewed changes

htmlCollapse refactoring

6ce1edd

Araq reviewed Jul 4, 2018

View reviewed changes

Quelklef added 2 commits July 16, 2018 15:17

Removed htmlCollapse

bea6768

Removed HTML comments

d738c67

Quelklef added 2 commits July 17, 2018 02:04

Fixed styling

cefea87

Add hover cue

66009d2

Araq merged commit f2b6efb into nim-lang:devel Jul 17, 2018

timotheecour mentioned this pull request Aug 23, 2018

[WIP] [doc] miscellaneous issues #8477

Closed

21 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make pragmas look clickable in docs #8176

Make pragmas look clickable in docs #8176

Quelklef commented Jul 1, 2018

Araq commented Jul 2, 2018

Quelklef commented Jul 2, 2018

Quelklef commented Jul 2, 2018 •

edited

dom96 commented Jul 2, 2018

Quelklef commented Jul 2, 2018 •

edited

Araq commented Jul 3, 2018

dom96 commented Jul 3, 2018

Quelklef commented Jul 3, 2018

Araq commented Jul 3, 2018

dom96 commented Jul 3, 2018

Quelklef commented Jul 3, 2018

Araq commented Jul 3, 2018 •

edited

dom96 commented Jul 3, 2018

Araq commented Jul 3, 2018

Quelklef commented Jul 3, 2018

dom96 commented Jul 3, 2018

Quelklef commented Jul 3, 2018 •

edited

kaushalmodi Jul 3, 2018

Quelklef Jul 3, 2018

kaushalmodi Jul 3, 2018

dom96 Jul 3, 2018

dom96 Jul 3, 2018

Araq Jul 4, 2018

Araq Jul 4, 2018

Araq Jul 4, 2018

Quelklef Jul 4, 2018

Quelklef Jul 4, 2018 •

edited

Quelklef commented Jul 16, 2018 •

edited

Quelklef commented Jul 17, 2018 •

edited

Quelklef commented Jul 17, 2018

Make pragmas look clickable in docs #8176

Make pragmas look clickable in docs #8176

Conversation

Quelklef commented Jul 1, 2018

Araq commented Jul 2, 2018

Quelklef commented Jul 2, 2018

Quelklef commented Jul 2, 2018 • edited

dom96 commented Jul 2, 2018

Quelklef commented Jul 2, 2018 • edited

Araq commented Jul 3, 2018

dom96 commented Jul 3, 2018

Quelklef commented Jul 3, 2018

Araq commented Jul 3, 2018

dom96 commented Jul 3, 2018

Quelklef commented Jul 3, 2018

Araq commented Jul 3, 2018 • edited

dom96 commented Jul 3, 2018

Araq commented Jul 3, 2018

Quelklef commented Jul 3, 2018

dom96 commented Jul 3, 2018

Quelklef commented Jul 3, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Quelklef Jul 4, 2018 • edited

Choose a reason for hiding this comment

Quelklef commented Jul 16, 2018 • edited

Quelklef commented Jul 17, 2018 • edited

Quelklef commented Jul 17, 2018

Quelklef commented Jul 2, 2018 •

edited

Quelklef commented Jul 2, 2018 •

edited

Araq commented Jul 3, 2018 •

edited

Quelklef commented Jul 3, 2018 •

edited

Quelklef Jul 4, 2018 •

edited

Quelklef commented Jul 16, 2018 •

edited

Quelklef commented Jul 17, 2018 •

edited