TiddlyWiki / TiddlyWiki5

A self-contained JavaScript wiki for the browser, Node.js, AWS Lambda etc.
https://tiddlywiki.com/
Other
7.99k stars 1.18k forks source link

[Inconsistency] Filtered transclusion syntax does not assign attributes to rendered HTML output #7797

Open pmario opened 11 months ago

pmario commented 11 months ago

While fixing [BUG] Filtered transclusion that is, much later, followed by curly brace #7701 -- I did find out that:

The docs in the js code for $:/core/modules/parsers/wikiparser/rules/filteredtranscludeblock.js says:

{{{ [tag[docs]] }}}
{{{ [tag[docs]] |tooltip}}}
{{{ [tag[docs]] ||TemplateTitle}}}
{{{ [tag[docs]] |tooltip||TemplateTitle}}}
{{{ [tag[docs]] }}width:40;height:50;}.class.class

In the code all the attributes are assigned to the list-like output node.

But only the "template" attribute is actually used by the rendered output All other attributes "tooltip", "style" and "class" are ignored


I think the problem is, that those additional attributes do not make too much sense, since it's not clear to which output element they should be added.

@Jermolene IMO it may make sense to:

a. make them available as variables to the "template" if there is one b. Ignore the issue and let them be as is for future use c: Comment the unused if clauses and activate them, when we need them in the future.

At the moment it's confusing for contributors and wasting device memory.

I would suggest option c:

@Jermolene -- What do you think?

pmario commented 11 months ago

The unused "tooltip" construction {{{ [tag[docs]] |tooltip||TemplateTitle}}} is inconsistent with transclusion parameters, because the order is different.

To be able to cleanly fix bug-https://github.com/Jermolene/TiddlyWiki5/issues/7701 I do suggest, that we completely remove the unused regexp and the unused code.

So we will be able to add stuff back if we should need it in the future.

@Jermolene -- I do need a decision about this one to be able to fix #7701. To be able to fix #7701 in a clean way we need at least remove match-group 4, the "style" section

Jermolene commented 11 months ago

Hi @pmario I would be inclined to agree that we should completely removed the unused parts of the regex and the associated code.

pmario commented 11 months ago

@Jermolene ... I did add 4 new tests for filtered transclusions "inline" and "block" mode.

So should I include the test with the PR that fixes issues #7701 and #7797

or

Should I create a separated PR that only contains the tests.

-> The problem will be, that the vercel CI will fail those tests

pmario commented 11 months ago

@Jermolene -- bump -- info needed

Jermolene commented 11 months ago

Hi @pmario in these situations I think it's clearer to add the tests in with the PR that includes the fix

pmario commented 8 months ago

The latest proposal from Jeremy is at: https://github.com/Jermolene/TiddlyWiki5/pull/7822#issuecomment-1826266336

Which can be changed to the following format, which gives us even more future possibilities.

vars ... can be any text except | -- even linebreaks are possible params ... similar to vars but with basically no limitations

{{{ [tag[docs]] }}}
{{{ [tag[docs]] | vars }}}
{{{ [tag[docs]] || TemplateTitle }}}
{{{ [tag[docs]] | vars || TemplateTitle }}}

{{{ [tag[docs]] }} params }
{{{ [tag[docs]] }} param:"value" | another="value" }

{{{ [tag[docs]] }} param:"value" | another="value" | any text we want 
including line-breaks }