Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Snippet inclusion removes some valid lines containing # characters #149

Open
agemooij opened this issue Sep 18, 2017 · 2 comments
Open

Snippet inclusion removes some valid lines containing # characters #149

agemooij opened this issue Sep 18, 2017 · 2 comments

Comments

@agemooij
Copy link

Hi

The @@snip feature appears to remove lines from snippets if they contain certain patterns related to # characters. It does this silently, without any warnings so users might not initially notice that their example snippets have been altered.

This is obviously related to support for nested snippets since it only apply to certain patterns of text that strongly resemble nested snippet markers.

Below is the test I've used so far to determine which lines will get filtered out of the snippet and which won't:

// Filtered out:
// 1 line with bare hash mark followed by a character: #a
// 2 line with bare hash mark followed by a character: abc#def
// 3 line with bare link with anchor: http://some.domain.com/foo#bar
// 4 line with markdown link with anchor: [IBM Cloud Object Storage](s3.md#bluemix-cloud-object-storage-with-s3-api )
//
// Not filtered out:
// 1 line with bare hash mark: #
// 2 line with quoted hash mark: "#"
// 3 line with quoted hash mark followed by a character: "#a"
// 4 line with quoted hash mark surrounded by characters: "abc#def"
// 5 line with bracket-quoted hash mark surrounded by characters: [abc#def]
// 6 line with bare link: http://some.domain.com/foo/bar
// 7 line with quoted link: http://some.domain.com/foo/bar
// 8 line with quoted link with anchor: "http://some.domain.com/foo#bar"
// 9 line with markdown link: [IBM Cloud Object Storage](s3.md)
// 10 line with quoted markdown link: "[IBM Cloud Object Storage](s3.md)"
// 11 line with quoted markdown link with anchor: "[IBM Cloud Object Storage](s3.md#bluemix-cloud-object-storage-with-s3-api)"
// 12 line with markdown link with anchor ending in bracket: [IBM Cloud Object Storage](s3.md#bluemix-cloud-object-storage-with-s3-api)

I'm not sure whether this can be entirely fixed but I think it is certainly counter-intuitive for most users that their links will be removed so perhaps the filter can at least try to detect links and URLs to prevent those from being filtered out?

/cc @olofwalker

@pvlugter
Copy link
Member

Yes, this is for nested snippets. Originally from a3000b8. Main use case is having full examples that wrap nested snippets but exclude the snippet marker comments.

A couple of approaches so that we don't match the fragment part of links:

Only allow comment markers and whitespace before the snippet marker. We could automatically determine // comments for scala and java sources, and allow it to be configurable. I think this was the approach we used for the sphinx version of this for the older akka docs. Can also have an overall flag for whether or not to exclude nested snippet markers. The old akka doc snippets could also exclude nested snippet sections altogether, with an option to insert a new comment instead.

Or we could have a special exclude marker. So that only if the line includes something like ###exclude-from-snippets will it be excluded. No chance of excluding the wrong things, but also makes it more verbose, where a nested snippet would need something like: // #nested-snippet ###exclude-from-snippets.

jroper added a commit to jroper/paradox that referenced this issue Feb 12, 2019
This improves the includes, so rather than the included markdown being
parsed at the parent parents render time, it's parsed and then included
in the AST straight after the parent page is parsed.

One consequence of this is that now the left hand page navigation works,
since the headers can be extracted from the included markdown. I think
it's also more robust all around.

In order to implement this, I had to create a new node, an IncludeNode,
which doesn't get parsed (the include is still a DirectiveNode), but
rather a post parse processing step converts the include DirectiveNode's
into IncludeNode's, by parsing the included file. The IncludeNode then
has the included files RootNode in it, along with the included path and
file object. Then, a serializer plugin, when it encounters the
IncludeNode, creates a new context based on the file that the node came
from, and renders the nodes children into that - this ensures that
included files can still have things like snips with paths relative to
the included file.

Doing all this exposed an issue - since we expect included markdown
files to include directives that may contain things that look like
labels, it would have a problem with lightbend#149. So I made the snippet parser
be able to turn on or off the filtering out of lines containing labels.
The include directive turns this off by default, while the snip and
other directives turn it on by default. Since that option now existed, I
also added support to all of the above directives to configure it, via a
`filterLabel` flag. This provides a work around, if not a fix, to lightbend#149.
@jroper
Copy link
Member

jroper commented Feb 12, 2019

A workaround, if not a fix, has been provided in #285, by allowing you to do this:

@@snip[example.scala](example.scala) { #example filterLabels=false }

jroper added a commit to jroper/paradox that referenced this issue Feb 14, 2019
This improves the includes, so rather than the included markdown being
parsed at the parent parents render time, it's parsed and then included
in the AST straight after the parent page is parsed.

One consequence of this is that now the left hand page navigation works,
since the headers can be extracted from the included markdown. I think
it's also more robust all around.

In order to implement this, I had to create a new node, an IncludeNode,
which doesn't get parsed (the include is still a DirectiveNode), but
rather a post parse processing step converts the include DirectiveNode's
into IncludeNode's, by parsing the included file. The IncludeNode then
has the included files RootNode in it, along with the included path and
file object. Then, a serializer plugin, when it encounters the
IncludeNode, creates a new context based on the file that the node came
from, and renders the nodes children into that - this ensures that
included files can still have things like snips with paths relative to
the included file.

Doing all this exposed an issue - since we expect included markdown
files to include directives that may contain things that look like
labels, it would have a problem with lightbend#149. So I made the snippet parser
be able to turn on or off the filtering out of lines containing labels.
The include directive turns this off by default, while the snip and
other directives turn it on by default. Since that option now existed, I
also added support to all of the above directives to configure it, via a
`filterLabel` flag. This provides a work around, if not a fix, to lightbend#149.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants