Skip to content

Commit

Permalink
Fix TOC macro with titles ending with double quotes
Browse files Browse the repository at this point in the history 
By almost entirely eliminating text subsitution, we can avoid the situations where special characters in tags or titles gets the macro confused.

These are quite intricate changes, and so I'd appreciate any help reviewing and testing, many thanks.

Fixes #3427
  • Loading branch information
@Jermolene
Jermolene committed Sep 10, 2018
1 parent 8743180 commit 587fe9d
Showing 1 changed file with 45 additions and 41 deletions.
86 changes: 45 additions & 41 deletions core/wiki/macros/toc.tid
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@ tags: $:/tags/Macro

\define toc-body(tag,sort:"",itemClassFilter,exclude,path)
<ol class="tc-toc">
<$list filter="""[all[shadows+tiddlers]tag[$tag$]!has[draft.of]$sort$] -[[$tag$]] $exclude$""">
<$vars item=<<currentTiddler>> path="""$path$/$tag$""" excluded="""$exclude$ -[[$tag$]]""">
<$set name="toc-item-class" filter="""$itemClassFilter$""" emptyValue="toc-item" value="toc-item-selected">
<$list filter="""[all[shadows+tiddlers]tag<__tag__>!has[draft.of]$sort$] -[<__tag__>] $exclude$""">
<$vars item=<<currentTiddler>> path={{{ [<__path__>addsuffix[/]addsuffix<__tag__>] }}} excluded="""$exclude$ -[<__tag__>]""">
<$set name="toc-item-class" filter=<<__itemClassFilter__>> emptyValue="toc-item" value="toc-item-selected">
<li class=<<toc-item-class>>>
<$list filter="[all[current]toc-link[no]]" emptyMessage="<$link><$view field='caption'><$view field='title'/></$view></$link>">
<<toc-caption>>
</$list>
<$macrocall $name="toc-body" tag=<<item>> sort="""$sort$""" itemClassFilter="""$itemClassFilter$""" exclude=<<excluded>> path=<<path>>/>
<$macrocall $name="toc-body" tag=<<item>> sort=<<__sort__>> itemClassFilter=<<__itemClassFilter__>> exclude=<<excluded>> path=<<path>>/>
</li>
</$set>
</$vars>
Expand All @@ -27,13 +27,15 @@ tags: $:/tags/Macro
\end

\define toc(tag,sort:"",itemClassFilter:" ")
<<toc-body tag:"""$tag$""" sort:"""$sort$""" itemClassFilter:"""$itemClassFilter$""">>
<$macrocall $name="toc-body" tag=<<__tag__>> sort=<<__sort__>> itemClassFilter=<<__itemClassFilter__>> />
\end

\define toc-linked-expandable-body(tag,sort:"",itemClassFilter,exclude,path)
<!-- helper function -->
<$set name="toc-state" value=<<qualify """$:/state/toc$path$-$(currentTiddler)$""">>>
<$set name="toc-item-class" filter="""$itemClassFilter$""" emptyValue="toc-item" value="toc-item-selected">
<$wikify name="toc-state" text="""

This comment has been minimized.

Sign in to view

Copy link
@pmario

pmario Nov 6, 2018

Contributor

wikify IMO is slow. There are faster possibilities.

This comment has been minimized.

Sign in to view

Copy link
@Jermolene

Jermolene Nov 6, 2018

Author Owner

Ouch! Apologies I did mean to come back to that. I think the best fix here is to expose qualify as an operator as well as a macro...
<$macrocall $name="qualify" title={{{ [[$:/state/toc]addsuffix<__path__>addsuffix[-]addsuffix<currentTiddler>] }}} />

This comment has been minimized.

Sign in to view

Copy link
@pmario

pmario Nov 6, 2018

Contributor

IMO we'll need to think about this again. This is a state tiddler. We should go for speed instead of readability.
""">
<$set name="toc-item-class" filter=<<__itemClassFilter__>> emptyValue="toc-item" value="toc-item-selected">
<li class=<<toc-item-class>>>
<$link>
<$reveal type="nomatch" state=<<toc-state>> text="open">
Expand All @@ -49,17 +51,19 @@ tags: $:/tags/Macro
<<toc-caption>>
</$link>
<$reveal type="match" state=<<toc-state>> text="open">
<$macrocall $name="toc-expandable" tag=<<currentTiddler>> sort="""$sort$""" itemClassFilter="""$itemClassFilter$""" exclude="""$exclude$""" path="""$path$"""/>
<$macrocall $name="toc-expandable" tag=<<currentTiddler>> sort=<<__sort__>> itemClassFilter=<<__itemClassFilter__>> exclude=<<__exclude__>> path=<<__path__>>/>
</$reveal>
</li>
</$set>
</$set>
</$wikify>
\end

\define toc-unlinked-expandable-body(tag,sort:"",itemClassFilter:" ",exclude,path)
<!-- helper function -->
<$set name="toc-state" value=<<qualify """$:/state/toc$path$-$(currentTiddler)$""">>>
<$set name="toc-item-class" filter="""$itemClassFilter$""" emptyValue="toc-item" value="toc-item-selected">
<$wikify name="toc-state" text="""
<$macrocall $name="qualify" title={{{ [[$:/state/toc]addsuffix<__path__>addsuffix[-]addsuffix<currentTiddler>] }}} />
""">
<$set name="toc-item-class" filter=<<__itemClassFilter__>> emptyValue="toc-item" value="toc-item-selected">
<li class=<<toc-item-class>>>
<$reveal type="nomatch" state=<<toc-state>> text="open">
<$button set=<<toc-state>> setTo="open" class="tc-btn-invisible tc-popup-keep">
Expand All @@ -74,32 +78,34 @@ tags: $:/tags/Macro
</$button>
</$reveal>
<$reveal type="match" state=<<toc-state>> text="open">
<$macrocall $name="toc-expandable" tag=<<currentTiddler>> sort="""$sort$""" itemClassFilter="""$itemClassFilter$""" exclude="""$exclude$""" path="""$path$"""/>
<$macrocall $name="toc-expandable" tag=<<currentTiddler>> sort=<<__sort__>> itemClassFilter=<<__itemClassFilter__>> exclude=<<__exclude__>> path=<<__path__>>/>
</$reveal>
</li>
</$set>
</$set>
</$wikify>
\end

\define toc-expandable-empty-message()
<<toc-linked-expandable-body tag:"""$(tag)$""" sort:"""$(sort)$""" itemClassFilter:"""$(itemClassFilter)$""" exclude:"""$(excluded)$""" path:"""$(path)$""">>
<$macrocall $name="toc-linked-expandable-body" tag=<<tag>> sort=<<sort>> itemClassFilter=<<itemClassFilter>> exclude=<<excluded>> path=<<path>>/>
\end

\define toc-expandable(tag,sort:"",itemClassFilter:" ",exclude,path)
<$vars tag="""$tag$""" sort="""$sort$""" itemClassFilter="""$itemClassFilter$""" excluded="""$exclude$ -[[$tag$]]""" path="""$path$/$tag$""">
<$vars tag=<<__tag__>> sort=<<__sort__>> itemClassFilter=<<__itemClassFilter__>> excluded="""$exclude$ -[<__tag__>]""" path={{{ [<__path__>addsuffix[/]addsuffix<__tag__>] }}}>
<ol class="tc-toc toc-expandable">
<$list filter="""[all[shadows+tiddlers]tag[$tag$]!has[draft.of]$sort$] -[[$tag$]] $exclude$""">
<$list filter="""[all[shadows+tiddlers]tag[$tag$]!has[draft.of]$sort$] -[<__tag__>] $exclude$""">
<$list filter="[all[current]toc-link[no]]" emptyMessage=<<toc-expandable-empty-message>> >
<$macrocall $name="toc-unlinked-expandable-body" tag="""$tag$""" sort="""$sort$""" itemClassFilter="""itemClassFilter""" exclude=<<excluded>> path=<<path>> />
<$macrocall $name="toc-unlinked-expandable-body" tag=<<__tag__>> sort=<<__sort__>> itemClassFilter="""itemClassFilter""" exclude=<<excluded>> path=<<path>> />
</$list>
</$list>
</ol>
</$vars>
\end

\define toc-linked-selective-expandable-body(tag,sort:"",itemClassFilter:" ",exclude,path)
<$set name="toc-state" value=<<qualify """$:/state/toc$path$-$(currentTiddler)$""">>>
<$set name="toc-item-class" filter="""$itemClassFilter$""" emptyValue="toc-item" value="toc-item-selected" >
<$wikify name="toc-state" text="""
<$macrocall $name="qualify" title={{{ [[$:/state/toc]addsuffix<__path__>addsuffix[-]addsuffix<currentTiddler>] }}} />
""">
<$set name="toc-item-class" filter=<<__itemClassFilter__>> emptyValue="toc-item" value="toc-item-selected" >
<li class=<<toc-item-class>>>
<$link>
<$list filter="[all[current]tagging[]limit[1]]" variable="ignore" emptyMessage="<$button class='tc-btn-invisible'>{{$:/core/images/blank}}</$button>">
Expand All @@ -117,16 +123,18 @@ tags: $:/tags/Macro
<<toc-caption>>
</$link>
<$reveal type="match" state=<<toc-state>> text="open">
<$macrocall $name="toc-selective-expandable" tag=<<currentTiddler>> sort="""$sort$""" itemClassFilter="""$itemClassFilter$""" exclude="""$exclude$""" path="""$path$"""/>
<$macrocall $name="toc-selective-expandable" tag=<<currentTiddler>> sort=<<__sort__>> itemClassFilter=<<__itemClassFilter__>> exclude=<<__exclude__>> path=<<__path__>>/>
</$reveal>
</li>
</$set>
</$set>
</$wikify>
\end

\define toc-unlinked-selective-expandable-body(tag,sort:"",itemClassFilter:" ",exclude,path)
<$set name="toc-state" value=<<qualify """$:/state/toc$path$-$(currentTiddler)$""">>>
<$set name="toc-item-class" filter="""$itemClassFilter$""" emptyValue="toc-item" value="toc-item-selected">
<$wikify name="toc-state" text="""
<$macrocall $name="qualify" title={{{ [[$:/state/toc]addsuffix<__path__>addsuffix[-]addsuffix<currentTiddler>] }}} />
""">
<$set name="toc-item-class" filter=<<__itemClassFilter__>> emptyValue="toc-item" value="toc-item-selected">
<li class=<<toc-item-class>>>
<$list filter="[all[current]tagging[]limit[1]]" variable="ignore" emptyMessage="<$button class='tc-btn-invisible'>{{$:/core/images/blank}}</$button> <$view field='caption'><$view field='title'/></$view>">
<$reveal type="nomatch" state=<<toc-state>> text="open">
Expand All @@ -143,49 +151,45 @@ tags: $:/tags/Macro
</$reveal>
</$list>
<$reveal type="match" state=<<toc-state>> text="open">
<$macrocall $name="""toc-selective-expandable""" tag=<<currentTiddler>> sort="""$sort$""" itemClassFilter="""$itemClassFilter$""" exclude="""$exclude$""" path="""$path$"""/>
<$macrocall $name="toc-selective-expandable" tag=<<currentTiddler>> sort=<<__sort__>> itemClassFilter=<<__itemClassFilter__>> exclude=<<__exclude__>> path=<<__path__>>/>
</$reveal>
</li>
</$set>
</$set>
</$wikify>
\end

\define toc-selective-expandable-empty-message()
<<toc-linked-selective-expandable-body tag:"""$(tag)$""" sort:"""$(sort)$""" itemClassFilter:"""$(itemClassFilter)$""" exclude:"""$(excluded)$""" path:"""$(path)$""">>
<$macrocall $name="toc-linked-selective-expandable-body" tag=<<tag>> sort=<<sort>> itemClassFilter=<<itemClassFilter>> exclude=<<excluded>> path=<<path>>/>
\end

\define toc-selective-expandable(tag,sort:"",itemClassFilter,exclude,path)
<$vars tag="""$tag$""" sort="""$sort$""" itemClassFilter="""$itemClassFilter$""" excluded="""$exclude$ -[[$tag$]]""" path="""$path$/$tag$""">
<$vars tag=<<__tag__>> sort=<<__sort__>> itemClassFilter=<<__itemClassFilter__>> excluded="""$exclude$ -[<__tag__>]""" path={{{ [<__path__>addsuffix[/]addsuffix<__tag__>] }}}>
<ol class="tc-toc toc-selective-expandable">
<$list filter="""[all[shadows+tiddlers]tag[$tag$]!has[draft.of]$sort$] -[[$tag$]] $exclude$""">
<$list filter="""[all[shadows+tiddlers]tag[$tag$]!has[draft.of]$sort$] -[<__tag__>] $exclude$""">
<$list filter="[all[current]toc-link[no]]" variable="ignore" emptyMessage=<<toc-selective-expandable-empty-message>> >
<$macrocall $name=toc-unlinked-selective-expandable-body tag="""$tag$""" sort="""$sort$""" itemClassFilter="""$itemClassFilter$""" exclude=<<excluded>> path=<<path>> >
<$macrocall $name=toc-unlinked-selective-expandable-body tag=<<__tag__>> sort=<<__sort__>> itemClassFilter=<<__itemClassFilter__>> exclude=<<excluded>> path=<<path>> >
</$list>
</$list>
</ol>
</$vars>
\end

\define toc-tabbed-selected-item-filter(selectedTiddler)
[all[current]field:title{$selectedTiddler$}]
\end

\define toc-tabbed-external-nav(tag,sort:"",selectedTiddler:"$:/temp/toc/selectedTiddler",unselectedText,missingText,template:"")
<$tiddler tiddler={{$selectedTiddler$}}>
<$tiddler tiddler={{{ [<__selectedTiddler__>get[text]] }}}>
<div class="tc-tabbed-table-of-contents">
<$linkcatcher to="$selectedTiddler$">
<$linkcatcher to=<<__selectedTiddler__>>>
<div class="tc-table-of-contents">
<$macrocall $name="toc-selective-expandable" tag="""$tag$""" sort="""$sort$""" itemClassFilter=<<toc-tabbed-selected-item-filter selectedTiddler:"""$selectedTiddler$""">>/>
<$macrocall $name="toc-selective-expandable" tag=<<__tag__>> sort=<<__sort__>> itemClassFilter="[all[current]field:title<__selectedTiddler__>]"/>
</div>
</$linkcatcher>
<div class="tc-tabbed-table-of-contents-content">
<$reveal state="""$selectedTiddler$""" type="nomatch" text="">
<$transclude mode="block" tiddler="$template$">
<$reveal state=<<__selectedTiddler__>> type="nomatch" text="">
<$transclude mode="block" tiddler=<<__template__>>>
<h1><<toc-caption>></h1>
<$transclude mode="block">$missingText$</$transclude>
</$transclude>
</$reveal>
<$reveal state="""$selectedTiddler$""" type="match" text="">
<$reveal state=<<__selectedTiddler__>> type="match" text="">
$unselectedText$
</$reveal>
</div>
Expand All @@ -194,8 +198,8 @@ tags: $:/tags/Macro
\end

\define toc-tabbed-internal-nav(tag,sort:"",selectedTiddler:"$:/temp/toc/selectedTiddler",unselectedText,missingText,template:"")
<$linkcatcher to="""$selectedTiddler$""">
<$macrocall $name="toc-tabbed-external-nav" tag="""$tag$""" sort="""$sort$""" selectedTiddler="""$selectedTiddler$""" unselectedText="""$unselectedText$""" missingText="""$missingText$""" template="""$template$"""/>
<$linkcatcher to=<<__selectedTiddler__>>>
<$macrocall $name="toc-tabbed-external-nav" tag=<<__tag__>> sort=<<__sort__>> selectedTiddler=<<__selectedTiddler__>> unselectedText=<<__unselectedText__>> missingText=<<__missingText__>> template=<<__template__>>/>
</$linkcatcher>
\end

10 comments on commit 587fe9d

@BurningTreeC
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Jermolene - I've tested this, I had also run into this problem testing tiddler titles and tag-names
Your new version works much better but still breaks if I use very bad titles and use them as tags, too - like """" this is a very !! bad // tiddler title"""

That's another reason why I was experimenting with the encodeuricomponent filters.

Now with your changes I was able to modify them in order to work with the """" this is a very !! bad // tiddler title""" example, using encoded titles and tags internally. It doesn't work with the bad-chars from the tiddler-title warning, but that was expectable.

@BurningTreeC
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here's the link: https://github.com/BurningTreeC/KeeBoord/blob/master/BurningTreeC/KeeBoord/macros/toc.tid

I actually modified the toc-selective-expandable macro only - and this is still the old version modified (iirc)

@Jermolene
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @BurningTreeC

breaks if I use very bad titles and use them as tags, too - like """" this is a very !! bad // tiddler title"""

I've been thinking about that, and concluding that we should refactor those widgets that take a text reference in an attribute to optionally allow the same attribute to be specified as separate tiddler title and field name attributes. Basically, I think text references should have been a parser thing, not a renderer thing.

@BurningTreeC
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Jermolene , thanks for replying!

the problem in the example tiddler is not just the !! but more the quotes. They can break a lot. Your new macro version fixes single quotes but it still breaks with triple quotes """

@Jermolene
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @BurningTreeC, I've posted a further fix.

@pmario
Copy link
Contributor

@pmario pmario commented on 587fe9d Nov 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BurningTreeC ... """" this is a very !! bad // tiddler title""" this title breaks the TW UI on everything that uses "tags" and "tagging"

@Jermolene
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @pmario is that a regression?

@BurningTreeC
Copy link
Contributor

@BurningTreeC BurningTreeC commented on 587fe9d Nov 6, 2018 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this title breaks the TW UI on everything that uses "tags" and "tagging"

@pmario - yes that's the current state ...

I've created a set of PR's which (together) should fix that (I'll have a look at the PR ID's and let you know) - the missing part is what @Jermolene proposed above: stateTitle, stateField and stateIndex attributes for button and reveal widgets (and ... ? ), which treat the passed strings as titles, without using state references (that fixes the !! and ## in titles)

is that a regression?

@Jermolene I don't think so, that kind of title makes problems on 5.1.17 as with the current state

@pmario
Copy link
Contributor

@pmario pmario commented on 587fe9d Nov 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @pmario is that a regression?

No the title is also a problem with the current version

@pmario
Copy link
Contributor

@pmario pmario commented on 587fe9d Nov 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @pmario is that a regression?

But I think this is: #3517

Please sign in to comment.