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

Deprecate <import> and <variable> #1353

Closed
5 tasks done
ang-zeyu opened this issue Sep 14, 2020 · 3 comments · Fixed by #1407
Closed
5 tasks done

Deprecate <import> and <variable> #1353

ang-zeyu opened this issue Sep 14, 2020 · 3 comments · Fixed by #1407

Comments

@ang-zeyu
Copy link
Contributor

ang-zeyu commented Sep 14, 2020

Is your request related to a problem?

Opening an issue to track: #751 (comment)

Describe the solution you'd like

  • allow <include><variable>...</variable></include> to override {% set / import %}
  • re-implement external json variables using a custom nunjucks tag
  • add deprecation warnings after the above for <import> and <variable>
  • add live preview for nunjucks sources (anytime)
  • finally, remove <import> and <variable> (v3.0)

Additional context

See discussion here #751 (comment)

@ang-zeyu
Copy link
Contributor Author

(1) and (4) have been merged in

for (2), I noticed that the current requirement of allowing variables in json to also reference other variables makes the re-implementation considerably more complex, but not impossible

I'm thinking that external variables should not be allowed to do so, since

  • the use case for sourcing external variables usually is some dataset that is independent of the markbind project in the first place
  • having nunjucks syntax in such standard files may not be rather user friendly - it requires the user to consider the placement of tag that sets the external variable carefully; any other variables declared before it in the page; any {% import %}s made, ..., which could all affect the result of the variables extracted from the file

Since its a new feature / replacement of an existing feature, we could do away with this (existing <variable type="json">s would still work till we deprecate that as well)

What do you think? @damithc
Are there worthwhile cases where we want nunjucks syntax to appear in such external files? (.json, .csv, .xlsx)

@damithc
Copy link
Contributor

damithc commented Nov 15, 2020

What do you think? @damithc
Are there worthwhile cases where we want nunjucks syntax to appear in such external files? (.json, .csv, .xlsx)

I guess we can live without it. So, go ahead. 👍

ang-zeyu added a commit to ang-zeyu/markbind that referenced this issue Nov 21, 2020
MarkBind page variables requires processing html before nunjucks and
markdown syntax.
This poses several architectural issues (MarkBind#1353) of simplicity,
performance, and extensibility of markdown syntax.

As another step in removing this prerequisite, let's reimplement
external json variables using a nunjucks extension.
ang-zeyu added a commit that referenced this issue Nov 25, 2020
MarkBind page variables requires processing html before nunjucks and
markdown syntax.
This poses several architectural issues (#1353) of simplicity,
performance, and extensibility of markdown syntax.

As another step in removing this prerequisite, let's reimplement
external json variables using a nunjucks extension.
@ang-zeyu
Copy link
Contributor Author

ang-zeyu commented Nov 28, 2020

will be getting a new release out tonight tomorrow with a few hotfixes (hopefully that's stable =P)

since that's all on the deprecation plate, I will be fast tracking the code to v3.0 from now on as well since full deprecation of this is necessary for the aims.

It'll mostly be backend changes requiring / including deprecation of some frontend user facing features:

  • Much simpler architecture
  • Much improved performance ~2x
  • Deprecate and change some things about plugins to facilitate the above
  • Deprecate old syntatic features (<modal title> -> <modal header>)
  • ...

@ang-zeyu ang-zeyu added this to Nice to Haves in Big Picture via automation Dec 6, 2020
@ang-zeyu ang-zeyu moved this from Nice to Haves to Done in Big Picture Dec 6, 2020
@ang-zeyu ang-zeyu removed this from Done in Big Picture Dec 6, 2020
ang-zeyu added a commit that referenced this issue Dec 6, 2020
Page variables and imports require processing html before nunjucks
and markdown. This poses several architectural issues of simplicity and
performance outlined in #1353.

Since these features now have viable alternatives, let's remove them.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants