[PATCH] D43505: [docs] Use literalinclude for most code in the Kaleidoscope tutorial to ensure the code always compiles and is up to date

[David Blaikie via llvm-commits]

On Wed, Feb 21, 2018 at 2:44 PM Aron Granberg via Phabricator <
reviews at reviews.llvm.org> wrote:

> Voxel added a comment.
>
>
>
> > 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.
>
> Looking at sphinx docs it seems like :lines: together with :start-after:
> only works properly in v1.6 or higher.
>

Ah, hmm. Wonder if anyone knows what version we have on bots, etc...


> > 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?
>
> 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.
>

*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)


>
> > 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)?
>
> 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.
>

Thanks!


>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D43505
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180221/a7a111c3/attachment.html>