TiddlyWiki / TiddlyWiki5

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

[BUG] Global variables set by $set widget don't work #8519

Open CodaCodr opened 4 weeks ago

CodaCodr commented 4 weeks ago

Describe the bug

See https://talk.tiddlywiki.org/t/best-widget-to-hold-a-static-variable/10412/11

Expected behavior

That the variables are made global as per docs at https://tiddlywiki.com/#SetWidget

To Reproduce

<$set name=r1 value=r1>
  <$set name=r2 value=r2>
    <$set name=r3 value=r3></$set>
  </$set>
</$set>

OR

<$set name=r1 value=r1>
  <$set name=r2 value=r2></$set>
  <$set name=r3 value=r3></$set>
</$set>

Or even... (would be nice)

<$set name=r1 value=r1></$set>
<$set name=r2 value=r2></$set>
<$set name=r3 value=r3></$set>

Screenshots

image

TiddlyWiki Configuration

tiddlywiki.com

Additional context

No response

pmario commented 4 weeks ago

If you do it the way it should be done -- it works

title: set-globals
tags: $:/tags/Global
code-body: yes

\define r1() r1
\define r2() r2
\procedure r3() r3
title: test-globals

r1: <<r1>>

r2: <<r2>>

r3: <<r3>>

image

CodaCodr commented 4 weeks ago

You're missing the point. As I laid out in the OP/report, the topic here is SetWidget and its documentation -- I'm not asking for a howto. Even the example given in the docs fails to work:

image

pmario commented 4 weeks ago

The last example needs a \whitespace trim. So the docs needs to be updated. eg: https://saqimtiaz.github.io/tw5-docs-pr-maker/#SetWidget

image

CodaCodr commented 4 weeks ago

SetWidget requires strict adherence to undocumented rules regarding whitespace sensitivities?

That's a bug too, IMO.

CodaCodr commented 4 weeks ago

I can't even bring myself to word it: "Oh and by the way, don't indent anything in this example, SetWidgets used to define global variables will break (silently)." 👎

pmario commented 4 weeks ago

@jermolene

I had a closer look at the code. The problem was introduced with commit: https://github.com/TiddlyWiki/TiddlyWiki5/commit/967e2b7fef0a7f1277e53187a412b6a190e72363 which fixes issue: https://github.com/TiddlyWiki/TiddlyWiki5/issues/7909

So fixing the problem with \procedure definitions we introduced a problem with the import of nested $set widgets.

Since there where no tests for "importing formatted set-widgets" the fix did not throw any errors while testing. see: https://github.com/TiddlyWiki/TiddlyWiki5/issues/8519#issuecomment-2292958185

pmario commented 4 weeks ago

There should be a new test similar to https://github.com/TiddlyWiki/TiddlyWiki5/blob/967e2b7fef0a7f1277e53187a412b6a190e72363/editions/test/tiddlers/tests/data/importvariables/WithSetWidgets2.tid but without \whitespace trim

@Jermolene -- This new test will also need to pass

pmario commented 4 weeks ago

The "WithSetWidget2" test should be the test without the whitespace pragma but there seems to be a copy paste error.

title: ImportVariables/WithSetWidgets2
description: Import variables defined with a set widget without whitespace pragma

ImportVariables/WithSetWidgets2 and ImportVariables/WithSetWidgets test tiddlers contain the exact same code

Jermolene commented 3 weeks ago

Thanks @CodaCodr @pmario – I think we'll need to fix this with yet another flag for the parser that configures whether the inheritance of whitespace settings is performed.

pmario commented 3 weeks ago

I think we'll need to fix this with yet another flag for the parser that configures whether the inheritance of whitespace settings is performed.

I did have a closer look at the $importvariables widget code. I think it has to be fixed there. If there are "text-nodes" that contains whitespace only, they should be ignored.

As soon as there are "real" text nodes or any other "node-type" import is not possible. -- As it is now.

IMO adding a new flag to the parser, without knowing anything about the tiddler to be imported, is not possible.

Jermolene commented 3 weeks ago

Hi @pmario the problem here is that importvariables must parse the tiddlers with whitespace trim so that it ignores whitespace between set widgets. However we don't want procedures to inherit the setting in this case unless the tiddler in which they appear has an explicit \whitespace trim. The behaviour in question is in the parser.