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

Add support for multiple snippet base directories via configurable placeholders #60

Merged
merged 2 commits into from
Jan 15, 2017

Conversation

sirthias
Copy link
Contributor

@sirthias sirthias commented Nov 9, 2016

Time has shown that the simple snippet.base_dir support that came in with 9afc38f isn't quite sufficient for my use-case (and likely others as well).

For example, I'd like to refer to files in the test branch of my doc project as well as actual library code in the main project sources. In order to do this in a DRY and concise fashion this patch adds support for defining placeholders for snippet base directories that can be easily referred to where required.

@eed3si9n
Copy link
Contributor

The use of dollar sign feels a bit ad-hoc to me. Could we just use @var[foo]?

@sirthias
Copy link
Contributor Author

Ultimately I don't have strong preference for a specific type of marker and picked the $ sign only because it's also the default "magic" char in StringTemplates.
Would it help if we went all-in-StringTemplate and simply required $root$ instead of $root?

@var[foo] would work as well, of course, but comes with at least 4 chars too many, which is why it wouldn't be my first choice.

@eed3si9n
Copy link
Contributor

eed3si9n commented Nov 11, 2016

With Paradox iiuc, the dollar-sign syntax only works within the *.st files, and it uses @var[foo] to lookup the StringTemplates on the markdown files.

@sirthias
Copy link
Contributor Author

Ok. Does anyone else (user, contributor or simply watcher) have an opinion on this?

@eed3si9n
Copy link
Contributor

let's ping @pvlugter

@sirthias
Copy link
Contributor Author

To sum up the question (for everyone caring to chip in):
What should be the demarcation of placeholders in snippet source strings?

So far we have these suggestions:

  1. $root/foo/bar.scala (root is the placeholder)
  2. $root$/foo/bar.scala
  3. @var[root]/foo/bar.scala

@sirthias
Copy link
Contributor Author

One more idea:
How about we simply adopt the markdown way of referring to "references":

[root]/foo/bar.scala

That should be clear and concise enough. And we could allow for escaping the brackets if necessary.

@pvlugter
Copy link
Member

The @@@ vars directive also uses $foo$ by default to align with StringTemplate.

I also don't have a strong preference for specific variable delimiters used, but do think that making it consistent and using the same defaults everywhere would be good.

If we go all-in with StringTemplate style, then it could even use StringTemplate itself to do the replacements, in the vars directive or in base dirs or elsewhere.

Or align with variable substitution standards that we often use, like Scala or Bash, and support both $foo and ${foo}. And update the vars directive to use this too. But then StringTemplate variables are different.

I think using markdown reference style of [root] is interesting.

Making it as consistent as possible across the different places that variable substitution is used, so that it's obvious and easy to remember, would be most important to me.

@sirthias
Copy link
Contributor Author

Ok, I think what is needed is a decision by the paradox core team on what the direction should be here.

@eed3si9n
Copy link
Contributor

Since I've been using Pamflet for docs, personally I'm comfortable with the idea of going all-in on using StringTemplate. So:

  • $root$/foo/bar.scala
  • In some other PR replace @var[root] notation with $root$

Related, is it possible to unify *.md and *.st files in themes?

@sirthias
Copy link
Contributor Author

Ok, sorry for not coming back earlier to this.
So I take it the decision is like @eed3si9n recommended:

  1. Demarcation of placeholders in snippet source strings will be done in the StringTemplate-style with a leading and closing $ character

  2. In another PR replace @var[root] notation with $root$

Related, is it possible to unify *.md and *.st files in themes?

I'm not sure this would really make sense. For markdown files it might be good to also run them through StringTemplate before feeding them to pegdown. But requiring this pipeline for all files (i.e. the normal HTML template without or only little markdown content) will have at least these two drawbacks:

  1. pegdown is much slower than StringTemplate when parsing HTML. In fact there are some known issues with exponential runtime for larger chunks of HTML. Not good!

  2. pegdown doesn't handle markdown inside of HTML tags (which was the reason behind Add directive for wrapping content in div or p elements ... #58). So, for pure HTML template files it doesn't actually add any real value.

@sirthias
Copy link
Contributor Author

sirthias commented Jan 12, 2017

Sorry for taking so long to get this done!
I just updated this PR as outlined in my last comment from Nov. 28th, i.e. placeholders in snippet source strings are now demarcated in StringTemplate-style with a leading and closing $ character.

I'll submit another PR which replaces the @var[root] notation with $root$ shortly.

@eed3si9n eed3si9n merged commit ac31ef0 into lightbend:master Jan 15, 2017
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

Successfully merging this pull request may close these issues.

3 participants