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

Conditional Shortcut Syntax #7710

Merged
merged 14 commits into from Oct 14, 2023
Merged

Conditional Shortcut Syntax #7710

merged 14 commits into from Oct 14, 2023

Conversation

Jermolene
Copy link
Owner

@Jermolene Jermolene commented Sep 1, 2023

This PR adds a new shortcut syntax for expressing if/then/else logic. It takes a different approach to #7691. Instead of adding a new widget, it adds a new parser rule for a shortcut syntax that expands to a series of nested <$list> widgets. For example:

\procedure test(animal)
<% if [<animal>match[Elephant]] %>
  It is an elephant
<% else %>
  <% if [<animal>match[Giraffe]] %>
    It is a giraffe
  <% else %>
    It is completely unknown
  <% endif %>
<% endif %>
\end

<<test "Giraffe">>

<<test "Elephant">>

<<test "Antelope">>

Behind the scenes, the <$list> widget is extended with two new features:

  • The option to specify the "emptyMessage" via a <$list-empty> widget within the list widget, instead of using the attribute, and similarly to use a <$list-template> widget to define the template
  • The option to limit the number of results returned from the filter

There are also some changes to the core parser to support the new syntax.

Progress

  • if-else-endif
  • elseif
  • Add support for block mode content
  • Decide on a name for the generic <%...%> syntax (ie the term we would also use for other constructions like switch/case that use the same syntax)
  • Improve documentation

@vercel
Copy link

vercel bot commented Sep 1, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
tiddlywiki5 ✅ Ready (Inspect) Visit Preview Oct 14, 2023 8:37am

@pmario
Copy link
Contributor

pmario commented Sep 1, 2023

Is the limit parameter needed for the if/else shortcut, or is it just an extension of the list-widget?

@pmario
Copy link
Contributor

pmario commented Sep 1, 2023

Ignore the last question. I found it in the code and in the parse- and widget-tree

@pmario
Copy link
Contributor

pmario commented Sep 1, 2023

I think I found a bug. I think the following example should result in 123 as a result.
I think <$list-template> and $list-empty should be optional.

Especially since emptyMessage is a "pain point" in the current list-widget implementation.

\whitespace trim

\procedure test(filter)
<$list filter=<<filter>> >
	<$text text=<<currentTiddler>>/>
	<$list-empty>
		None!
	</$list-empty>
</$list>
\end

<<test "1 2 3">>

image

@rmunn
Copy link
Contributor

rmunn commented Sep 4, 2023

Here's one test I wrote for the <$if> widget, which I removed from the PR because it was spamming the console log, but which might be useful in testing the {@ if @} syntax as well. It uses <$log> widgets to make sure that only one branch of the whole if...elseif...elseif...else chain is ever wikified:

it("if widget should short-circuit child rendering, so only one child ever gets rendered", function() {
    var wiki = new $tw.Wiki();
    // Construct four widget nodes
    var text = "<$if filter='[[1]compare:number:eq[2]]'><$then>yes<$log message='then' /></$then><$elseif filter='[[3]compare:number:eq[4]]'>maybe<$log message='elseif1' /></$elseif><$elseif filter='[[5]compare:number:eq[6]]'>another<$log message='elseif2' /></$elseif><$else>no<$log message='else' /></$else></$if>";
    var falsey = text;
    var elseif2 = falsey.replace("eq[6]]","eq[5]]");
    var elseif1 = elseif2.replace("eq[4]]","eq[3]]"); // Both second and first elseifs are true, first one wins
    var truthy = elseif1.replace("eq[2]]","eq[1]]"); // The "main" if condition and both elseifs are true, "main" condition wins
    var widgetNodeF = createWidgetNode(parseText(falsey,wiki),wiki);
    var widgetNodeT = createWidgetNode(parseText(truthy,wiki),wiki);
    var widgetNodeE1 = createWidgetNode(parseText(elseif1,wiki),wiki);
    var widgetNodeE2 = createWidgetNode(parseText(elseif2,wiki),wiki);
    // Render four widget nodes to the DOM
    console.log('Rendering F');
    var wrapperF = renderWidgetNode(widgetNodeF);
    console.log('Rendering T');
    var wrapperT = renderWidgetNode(widgetNodeT);
    console.log('Rendering E1');
    var wrapperE1 = renderWidgetNode(widgetNodeE1);
    console.log('Rendering E2');
    var wrapperE2 = renderWidgetNode(widgetNodeE2);
    // Test the rendering of all four
    expect(wrapperF.innerHTML).toBe("<p>no</p>");
    expect(wrapperT.innerHTML).toBe("<p>yes</p>");
    expect(wrapperE1.innerHTML).toBe("<p>maybe</p>");
    expect(wrapperE2.innerHTML).toBe("<p>another</p>");
});

Expected output if all goes well is "Rendering F" followed by exactly one <$log> output, then "Rendering T" followed by exactly one <$log> output, and so on for E1 and E2. I chose the <$log> widget because that was the only way I could see to have something trigger when the widget is rendered; everything else (like action-setfield) only triggers on a button press. I would have preferred to test the widget by having an action change a field or set a variable, then checking the value of that variable after rendering the <$if> widget... but I couldn't find a way to do that. So I don't have an automated "this is short-circuiting properly" test, just one that spams the console log and then I look at the console log to make sure it printed the right thing.

Anyway, I hope this is somewhat helpful in building a similar test for the list widgets built with the {@ if @} syntax, to make sure they short-circuit properly and don't even build the widgets for the parse tree branches that aren't selected.

@pmario
Copy link
Contributor

pmario commented Sep 4, 2023

The parse tree can be checked directly. See eg: test-wikitext-parser.js

@rmunn
Copy link
Contributor

rmunn commented Sep 4, 2023

Thanks, but what I'm trying to test is that certain parts of the parse tree do not get turned into widgets. Checking the parse tree won't help with that.

@pmario
Copy link
Contributor

pmario commented Sep 4, 2023

It's also possible to check the parse-tree, which only contains the widgets, which will be active

Click for: test-code and widget-tree preview
\procedure test(animal)
{% if [<animal>match[Elephant]] %}
  It is an elephant
{% elseif [<animal>match[Giraffe]] %}
    It is a giraffe
{% else %}
    It is completely unknown
{% endif %}
\end

<<test "Giraffe">>

image

@Jermolene
Copy link
Owner Author

2411ebc changes the syntax from {%if%} to <%if%> - see the discussion here https://talk.tiddlywiki.org/t/proposed-if-widget/7882/64

@kookma
Copy link
Contributor

kookma commented Sep 8, 2023

I am not sure if this question is relevant, but the underlaying $list widget shows the below error when an empty line is inserted:

\define filter() [tag[HelloThere]]


<$list filter=<<filter>>>

	<$list-template>
		<$text text=<<currentTiddler>>/>
	</$list-template>
	<$list-empty>
		None!
	</$list-empty>
</$list>

shows

Undefined widget 'list-template' Undefined widget 'list-empty' Undefined widget 'list-join'

but without extra empty line, it works (see below)

\define filter() [tag[HelloThere]]


<$list filter=<<filter>>>
	<$list-template>
		<$text text=<<currentTiddler>>/>
	</$list-template>
	<$list-empty>
		None!
	</$list-empty>
</$list>

It seems adding empty line between $list and $list-template, and $list-empty shows the above error.

@pmario
Copy link
Contributor

pmario commented Sep 11, 2023

@Jermolene ... Did you have a look at this one: #7710 (comment) -- I think it's a bug

@Jermolene
Copy link
Owner Author

I think I found a bug

Hi @pmario that is a case that I wasn't sure how to handle: what to do if the <$list-empty> widget is found, but not the <$list-template> widget. Perhaps in such cases it should behave as if there was an empty <$list-template> widget.

@Jermolene
Copy link
Owner Author

Here's one test I wrote for the <$if> widget, which I removed from the PR because it was spamming the console log, but which might be useful in testing the {@ if @} syntax as well.

Thanks @rmunn that is helpful.

I would have preferred to test the widget by having an action change a field or set a variable, then checking the value of that variable after rendering the <$if> widget... but I couldn't find a way to do that.

I'm not sure that it is enough for our purposes here, but the new(ish) wiki-based test framework supports actions that are performed after the initial rendering to force a refresh.

title: Transclude/Variable/Refreshing
description: Transcluding and refreshing a function
type: text/vnd.tiddlywiki-multiple
tags: [[$:/tags/wiki-test-spec]]
title: Output
\function list-join(filter, sep:", ") [subfilter<filter>join<sep>]
<$tiddler tiddler="TestData">
<<list-join "[enlist{!!items}]">>
</$tiddler>
+
title: TestData
+
title: Actions
<$action-setfield $tiddler="TestData" items={{{ [range[10]join[ ]] }}}/>
+
title: ExpectedResult
<p>1, 2, 3, 4, 5, 6, 7, 8, 9, 10</p>

@Jermolene
Copy link
Owner Author

I am not sure if this question is relevant, but the underlaying $list widget shows the below error when an empty line is inserted:

Thanks @kookma this is an interesting one. At the moment, the list widget only looks at its immediate children when searching for the <$list-template> and <$list-empty> pseudo widgets. The extra blank line causes the content of the list widget to be parsed in block mode, which means that its content is wrapped in <p> tag, which means that the <$list-template> and <$list-empty> are no longer immediate children of the list widget. Thus, they are not found, and the list widget assumes that the entire content of the list widget is the item template. The undefined widget errors are raised because the <$list-template> and <$list-empty> widget are only pseudo widgets, and are not recognised outside of their role within the list widget.

One possible fix would be to do a recursive search for the <$list-template> and <$list-empty> pseudo widgets, so that they would be found even if they are wrapped in a <p> tag.

The trouble with that approach is that it might have unexpected consequences. For example, here the inner list widget will always evaluate to an empty list, and thus its content will never be rendered. However, if we supported recursive searching then the outer list widget would still find the <$list-template> and <$list-empty> widgets. Now, we could make a special case for not searching within nested list widgets, but that doesn't allow for other widgets that may have similar functionality.

<$list filter="...">
<$list filter="[[never]match[something]]">
<$list-template>
This is a template
</$list-template>
</$list>
</$list>

Perhaps the best we can do is to make HTML elements explicitly transparent when searching for <$list-template> and <$list-empty> widgets, but make all other widgets be opaque.

We'd still be left with the problem that a stray <$list-template> or <$list-empty> widgetswidget will display an undefined widget error. We could improve the situation by defining those widgets as an alias of the existing <$error> widget.

@rmunn
Copy link
Contributor

rmunn commented Sep 13, 2023

Hi @pmario that is a case that I wasn't sure how to handle: what to do if the <$list-empty> widget is found, but not the <$list-template> widget. Perhaps in such cases it should behave as if there was an empty <$list-template> widget.

Unless there's a reason not to, I'd suggest handling that case in line with the existing behavior: "When no template is specified, the body of the ListWidget serves as the item template." If there's a <$list-empty> widget it's removed, then the rest of the body is treated as if it was in the <$list-template> widget.

As for the block-mode parsing issue that @kookma found,

One possible fix would be to do a recursive search for the <$list-template> and <$list-empty> pseudo widgets, so that they would be found even if they are wrapped in a <p> tag.

I'd suggest being slightly more limited, and just do a search for either children, or grandchildren where the immediate parent is a <p> tag. That should be enough to cover this scenario. And since the <$list-template> documentation already says that it should be an immediate child of the ListWidget, there's no need to test for cases like "What if it's inside a <strong> or <em> tag?" because the documentation says not to do that, so anyone who does that shouldn't be surprised that it doesn't work. (The extra <p> element wrapped around it is a bit of an exception, because it's invisible in the WikiText syntax and so people wouldn't expect that they would be adding an element merely by inserting a blank line).

@linonetwo
Copy link
Contributor

linonetwo commented Sep 23, 2023

I found it hard to type <% because after typing <, it is hard to move eyes and right hands across have of keyboard to find % (left hand is always holding Shift!)

Can it be two same char just like {{ [[ << ? For example (( or [[[

And if this will be alias for some widgets, I want to add effect widgets described in https://talk.tiddlywiki.org/t/config-tiddler-title-for-default-light-dark-palette-e-g-config-palette-default-dark/8071/22?u=linonetwo

@Jermolene
Copy link
Owner Author

Hi @linonetwo

Can it be two same char just like {{ [[ << ? For example (( or [[[

Double parenthesis is currently unused and so might be a promising choice:

\procedure test(animal)
((if [<animal>match[Elephant]] ))
  It is an elephant
((else))
  ((if [<animal>match[Giraffe]] ))
    It is a giraffe
  ((else))
    It is completely unknown
  ((endif))
((endif))
\end

<<test "Giraffe">>

<<test "Elephant">>

<<test "Antelope">>

@pmario
Copy link
Contributor

pmario commented Sep 25, 2023

Double parenthesis is currently unused and so might be a promising choice:

I think standard double parenthesis are way to common in prose text. Especially if someone explains an algorithm with "pseudo syntax" and IMO it's very close to LISP which may cause confusion.

@linonetwo
Copy link
Contributor

are way to common in prose text

This is not common in daily writing, if encounter this, maybe can use """ to wrap the.

explains an algorithm

Also can wrap them with ``` code block

I think this syntax won't work in a block.

@linonetwo
Copy link
Contributor

linonetwo commented Sep 25, 2023

@Jermolene Have you considered /if /endif syntax? Currently fnprocdef import macrodef are all using this syntax and each has its own parser, so there can also be a new parser for it. (And there may be more in the future).

One drawback is these pragma can only be used on top of tiddler.


Already discussed in #7680

@Jermolene
Copy link
Owner Author

I think standard double parenthesis are way to common in prose text.

Really? I would be interested to understand why that might be. I would not expect to encounter double parenthesis in general prose, but only in technical contextx.

Especially if someone explains an algorithm with "pseudo syntax" and IMO it's very close to LISP which may cause confusion.

Hmmm those needs are readily catered for with the codeblock syntax, or the triple double quote syntax. I don't think that a rare use case like that should drive the choice of syntax that would affect far more users.

@Jermolene Have you considered /if /endif syntax? Currently fnprocdef import macrodef are all using this syntax and each has its own parser, so there can also be a new parser for it. (And there may be more in the future).

One drawback is these pragma can only be used on top of tiddler.

There are two problems: backslash at the start of a line indicates a pragma, and pragmas have the restriction that they must precede ordinary parsed text. The other issue is that there is no lineline syntax for pragmas.

@rmunn
Copy link
Contributor

rmunn commented Sep 27, 2023

I think standard double parenthesis are way to common in prose text.

Really? I would be interested to understand why that might be. I would not expect to encounter double parenthesis in general prose, but only in technical contextx.

I have sometimes seen double parentheses used as a way of making a note to self in some text to come back and fix later. E.g,, in a novel in progress, the author might write:

The detective examined the bullet wounds. "Looks like the victim was shot with a .22 pistol," he said. "Either a ((model)) or a ((model)) would be my guess, but forensics should tell us more." ((TODO: Figure out a couple of plausible models)).

Then later when he's done writing the scene, he'd go online and do a bit of research and decide which models are plausible for a criminal to be able to get hold of easily in ((name of town)). I know a couple people (online and in real life) who have written books, and some of them have mentioned that they use this approach a lot so they don't have to interrupt the flow of words to go and do fiddly research; they leave the research until a time when inspiration isn't hitting.

So I would agree with avoiding double parentheses for anything. It's used fairly often as far as I've seen, precisely because it doesn't occur in normal text but it's extremely easy to type, and easy to search for later when you're looking for your notes-to-self.

@linonetwo
Copy link
Contributor

backslash at the start of a line indicates a pragma

Is it hard to relax this limitation? Or use \\ for \\if ?

@Jermolene
Copy link
Owner Author

I have sometimes seen double parentheses used as a way of making a note to self in some text to come back and fix later. E.g,, in a novel in progress, the author might write:

I would actually take the fact that double parenthesis is used as ad-hoc markup as validation: it demonstrates that it is a construction that is rarely used in prose, and is not visually distracting.

The detective examined the bullet wounds. "Looks like the victim was shot with a .22 pistol," he said. "Either a ((model)) or a ((model)) would be my guess, but forensics should tell us more." ((TODO: Figure out a couple of plausible models)).

Of course, in TiddlyWiki we would suggest using {{model}}.

Is it hard to relax this limitation? Or use \\ for \\if ?

It is a limitation in the sense that it needs to be consistent with the new syntax. A syntax that closely resembles the pragma syntax but has subtly different rules seems like it would be confusing.

@Jermolene
Copy link
Owner Author

I've cherry picked the list widget enhancements into a new PR in #7784

Jermolene added a commit that referenced this pull request Oct 14, 2023
@Jermolene Jermolene marked this pull request as ready for review October 14, 2023 08:35
@Jermolene Jermolene merged commit b7562f0 into master Oct 14, 2023
4 checks passed
@CodaCodr
Copy link
Contributor

<$let tag=HelloThere>
''<% if [tag<tag>] %>
There are tiddlers tagged with <$text text=<<tag>>/>
<% else%>
There are no tiddlers tagged with <$text text=<<tag>>/>
<% endif %>''
</$let>

What's interesting here is the styling via '' works. For example, this doesn't work:

''<$list filter="1" />''

But, anyway...

Decide on a name for the generic <%...%> syntax (ie the term we would also use for other constructions like switch/case that use the same syntax)

Because the syntax can be viewed as subsuming <$list> widgets, it can be seen as indirect syntax. In contrast, because it can be claimed to be more expressive, it can be seen as directed syntax.

Not sure if either is a great name, though. 😕

@Jermolene
Copy link
Owner Author

What's interesting here is the styling via '' works. For example, this doesn't work:

I can't test your example now, but I would not have expected it to work.

But, anyway...

Decide on a name for the generic <%...%> syntax (ie the term we would also use for other constructions like switch/case that use the same syntax)

Because the syntax can be viewed as subsuming <$list> widgets, it can be seen as indirect syntax. In contrast, because it can be claimed to be more expressive, it can be seen as directed syntax.

Not sure if either is a great name, though. 😕

I've ended up going for "shortcut syntax", meaning it's a shortcut for the underlying widget syntax.

@CodaCodr
Copy link
Contributor

I've ended up going for "shortcut syntax", meaning it's a shortcut for the underlying widget syntax.

IMO, that's not a good reason for choosing the name. Fresh eyes don't know the background or (perhaps) understand the reasoning. Fresh eyes certainly don't need to know that baggage to use it.

If, as you mentioned, there are more shortcuts to come, you (we) need a better name, IMO.

@Jermolene
Copy link
Owner Author

Hi @CodaCodr the requirement here isn't to come up with a word or phrase that will be self explanatory. We just need to name the concept, which means a label that is unique/distinctive within the context. In this case, we have an opportunity to choose a name that leads the user towards a deeper understanding of what is going on.

@pmario
Copy link
Contributor

pmario commented Oct 21, 2023

Try this. IMO your second example shows a CSS link so the formatting is overwritte.

''<$list filter="1" > <<currentTiddler>> </$list>''

Have a closer look to the parse-tree preview. You'll see it's similar to the first example

@CodaCodr
Copy link
Contributor

I strongly urge you to reconsider (the effect on fresh eyes). This is the kind of thing that can foster cognitive overload in a newcomer -- "will I ever understand this stuff?"

That's not to say that the background/underlying "truth" shouldn't be made available and attention drawn to it, but elsewhere, perhaps behind "Learn more" or similar. But to place that in its name? I don't need to be reminded (nor, least of all, do you) but a newcomer?

And does it scale? 50 new pence. Windows NT.

I'm harping on because this stuff matters. :-|

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.

None yet

6 participants