<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Wed, Feb 21, 2018 at 2:44 PM Aron Granberg via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Voxel added a comment.<br>
<br>
<br>
<br>
> Though the ":lines: X, Y" doesn't seem to do the right thing for me (using sphinx-build v1.5.6 on my Ubuntu system). Any of the code samples that use the 'lines:' property end up empty for me.<br>
<br>
Looking at sphinx docs it seems like :lines: together with :start-after: only works properly in v1.6 or higher.<br></blockquote><div><br>Ah, hmm. Wonder if anyone knows what version we have on bots, etc... <br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">> Though honestly, I'm thinking maybe it'd be better to not 'lines:' property anyway. Perhaps it's better to have the start/end comments in the source so when code is refactored, line wrapped, etc, the line counts don't end up becoming out of date?<br>
<br>
I agree that line counts are not ideal, unfortunately (also related to your other comment) it does not seem possible to strip the tags using sphinx, and in several places in the tutorial there are overlapping code blocks which would mean that tags would show up inside those code blocks, and that's not ideal either.<br></blockquote><div><br>*nod* Fair, yeah. I'd be OK using counts in those situations, I guess. Not a lot else to do. (could use something in reference to the source code in the end-after text, instead of a dedicated identifier/separator? Though I guess in many cases the 'end' is maybe a '}' which isn't sufficiently unique)<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
> Nitpick: Might be good to keep any whitespace that was present before (in some cases an empty line has been replaced with two comments (the end of one region and the start of the next) - perhaps keep the empty line between those two comments, for readibility and to reduce the noise in this diff)?<br>
<br>
Ah, true. I tried to keep it, but it looks like I missed a few places. I'll see if I can tweak the diff a bit.<br></blockquote><div><br></div><div>Thanks!</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<br>
Repository:<br>
  rL LLVM<br>
<br>
<a href="https://reviews.llvm.org/D43505" rel="noreferrer" target="_blank">https://reviews.llvm.org/D43505</a><br>
<br>
<br>
<br>
</blockquote></div></div>