Timeline for 5.1.23 release?

[saqimtiaz]

@Jermolene perhaps now would be a good time to set a timeline for the 5.1.23 release so we can co-ordinate our efforts better?

I don't believe we have any blocking issues that need resolving.

We should decided if we want to add an upgrader module for dealing with the journal tags: #4721

There are a couple of almost ready PRs that would be good to include but need a bit more work/testing, but even there a timeline might push things along:

• Fix filesystem adaptor #5113
• Add actions parameter to radio-widget #5026
• Add Range actions #5052
• [BUG] Remove TiddlySpot tab from Control Panel? #5120

Is there anything else that I am not thinking of @pmario @BurningTreeC @joshuafontany ?

[BurningTreeC]

@saqimtiaz we should definitely add an upgrader module for dealing with #4721

[Jermolene]

Hi @saqimtiaz @BurningTreeC I was also thinking about the four tickets that @saqimtiaz mentions in the OP, but I wasn't sure what we'd decided about #4721, and will leave a comment there.

My main task for this weekend is to bring the release note up to date and hopefully rearrange it a bit as the current categories aren't working so well with so many new items.

If all goes well with that, I should be able to send out the usual last call on the mailing list on Monday, and then perhaps release next weekend.

[saqimtiaz]

@Jermolene perhaps you could also check if you can reproduce this issue on OSX:
6c98bb7#commitcomment-44594409

I was not able to reproduce it in Chrome on OSX, Windows or Linux.

[Jermolene]

I'm just pinging other contributors of this release to let you all know our plans to finalise v5.1.23 shortly.

This is a good time to test your plugins etc against the prerelease, and bump any PRs you'd like to see merged for v5.1.23.

@adithya-badidey
@Arlen22
@bimlas
@BramChen
@BurningTreeC
@danielo515
@ento
@favadi
@fkohrt
@flibbles
@gera2ld
@ibnishak
@idotobi
@jdangerx
@jjduhamel
@Kamal-Habash
@Marxsal
@mocsa
@NicolasPeton
@passuf
@pmario
@rmunn
@saqimtiaz
@twMat
@default-kramer

[rmunn]

Thanks for nudging me to create #5144 finally, which should probably be merged in before 5.1.23 goes out. (See #5051 (comment) for the reason behind #5144).

[rmunn]

Knowing that the 5.1.23 release is near has also spurred me to finally create #5149, reviving the documentation work done by @00SS and Watt on how to use anchor links in TiddlyWiki (see #3811 and #3836, as well as #4947, for background). Would be nice to get this into the release since the double-hash behavior is now specifically checked for in the code and guaranteed to work.

[rmunn]

I'd also like #5151 (and PR #5152 which would fix it) to be considered.

[saqimtiaz]

@rmunn looks we are missing some documentation for the suffix option added to the trim operator.
Could you please have a look?
https://tiddlywiki.com/prerelease/#trim%20Operator

[rmunn]

I've added documentation for the suffix to the trim operator in the #5147 PR since it was already open. As I was writing the documentation, it occurred to me that the values I chose for the suffix aren't necessarily ideal, and now's the time to change them if I'm going to change them, before we release and they're locked in. The values I chose were prefix to trim from the front, and suffix to trim from the end, to parallel the removeprefix and removesuffix operators. But now I think there might be some better options. Could I get some opinions?

Possible suffixes for trim operator:

• prefix / suffix to trim from front and back
• front / back to trim from front and back
• start / end to trim from front and back
• Some or all of the above (e.g., allow either front or start to mean trim from front, and either back or end to mean trim from back).

Thoughts? I'm leaning towards front and back myself instead of prefix and suffix, but want to see what other people think is most intuitive.

[saqimtiaz]

@rmunn normally I would strongly favour more layman oriented terms like start/end, but in this case I feel that consistency with addprefix/removeprefix etc is more important.

Otherwise I would favour start/end over front/back

[Jermolene]

Thanks @rmunn @saqimtiaz I'm happy with prefix/suffix here

[pmario]

NEW PR: Add actions parameter to range-widget + docs! #5158

Old: #5052 Add range actions

[rmunn]

Okay, I'll leave it as-is with prefix/suffix being valid suffix values for trim. In which case no further action is needed on the trim operator.

Which means I've finished submitting all the PRs I want to get in before the release. Unless there's another bug or missed bit of documentation that I need to fix, all my stuff is in.

[rmunn]

There was one bit I missed in the trim suffix documentation: it should mention that the suffix was introduced in 5.1.23. #5161 has that change.

[saqimtiaz]

Update: everything mentioned here as needing consideration has been resolved.

[Jermolene]

Thanks @saqimtiaz. I want to explore #5188 a little more but otherwise I agree we seem to be in good shape.

[saqimtiaz]

@Jermolene yes agreed on #5188. My comment was not meant to suggest that we should proceed to release, rather just a status update on everything mentioned here.

You may also want to take a look at this feedback from the Google group.

[Jermolene]

Thanks @saqimtiaz I'd forgotten about the issue @bimlas raised with custom search tabs.

[pmario]

I've no problem, if the release needs more time. I'll need it for my plugins too ;)

[rmunn]

I expect to get in a PR for #5189 within the next 24 hours; if I don't think I'll make that deadline, I'll let you know.

Edit: Done. PR is #5191.

[saqimtiaz]

I think it might be prudent to not release 5.1.23 until after the weekend. There will be users that are unable to test during the working week, and there are still small issues being reported.

[BurningTreeC]

Thanks @saqimtiaz I'd forgotten about the issue @bimlas raised with custom search tabs.

That issue evolves from my changes regarding keyboard driven tab switching. I couldn't find another way to do it than by adding the configTiddler variable.

[bimlas]

(we are talking about this issue)

I couldn't find another way to do it than by adding the configTiddler variable.

There is no problem with that. What is strange is that configTiddler is available in $:/core/ui/DefaultSearchResultList but not in a custom search results, although both are transcluded by $:/core/ui/SideBarSegments/search, in which configTiddler is defined.

//Later...//

Ok, I've got it: The title of the currently selected search results tiddler is the value of configTiddler. Based on this, you may need to change the values of primaryListFilter and secondaryListFilter so that if they are not available in the current search results tiddler, retrieve them from the default search tiddler (the really default, not the configured).

<$vars primaryListFilter={{{ [<configTiddler>has[first-search-filter]] [[$:/core/ui/DefaultSearchResultList]] +[get[first-search-filter]] }}} secondaryListFilter={{{ [<configTiddler>has[second-search-filter]] [[$:/core/ui/DefaultSearchResultList]] +[get[second-search-filter]] }}}>

In the search results tiddler, these variables should be used instead of the configTiddler variable, but if I modify the code of default search results this way, it doesn't take into account the text I'm looking for, but lists all the tiddlers.

I’m not at all sure if what’s written above really takes the code in the right direction, but see if it gives someone an idea.

[BurningTreeC]

Thanks @bimlas , I'll have a look

[bimlas]

I found another problem that worked in 5.1.22 but not in 5.1.23: if you create a tiddler from the search text with the following search results tiddler, it will delete the text from search input (text of $(searchTiddler)$) in 5.1.22, not in 5.1.23.

\define searchResultList()
  <$button class="tc-btn-invisible">
    <$action-sendmessage $message="tm-new-tiddler" title={{$(searchTiddler)$}}/>
    <$action-log searchTiddler="$(searchTiddler)$" textBefore={{$(searchTiddler)$}}/>
    <$action-setfield $tiddler="$(searchTiddler)$" text=""/>
    <$action-log searchTiddler="$(searchTiddler)$" textAfter={{$(searchTiddler)$}}/>
    {{$:/core/images/new-button}}
    <$text text=<<currentTiddler>>/>
  </$button>
\end

<<searchResultList>>

In the console, it appears that the text is being deleted, but the search input still shows the previous text, and when I click it, the popup does not pop up, so it really feels like there is no text being searched for, but the input still contains it. Tried out on Vivaldi (Chromium based browser) and Firefox, same results.

https://imgur.com/a/YpBKB0R

[bimlas]

Just tried out to replace "action-setfield" by "action-deletetiddler", got the same results.

[BurningTreeC]

@bimlas that might be because of the new search mechanism not updating or deleting a state Tiddler. I'll have a closer look this evening

[rmunn]

#5191 is the PR for #5189.

[BurningTreeC]

I found another problem that worked in 5.1.22 but not in 5.1.23: if you create a tiddler from the search text with the following search results tiddler, it will delete the text from search input (text of $(searchTiddler)$) in 5.1.22, not in 5.1.23.

\define searchResultList()
  <$button class="tc-btn-invisible">
    <$action-sendmessage $message="tm-new-tiddler" title={{$(searchTiddler)$}}/>
    <$action-log searchTiddler="$(searchTiddler)$" textBefore={{$(searchTiddler)$}}/>
    <$action-setfield $tiddler="$(searchTiddler)$" text=""/>
    <$action-log searchTiddler="$(searchTiddler)$" textAfter={{$(searchTiddler)$}}/>
    {{$:/core/images/new-button}}
    <$text text=<<currentTiddler>>/>
  </$button>
\end

<<searchResultList>>

In the console, it appears that the text is being deleted, but the search input still shows the previous text, and when I click it, the popup does not pop up, so it really feels like there is no text being searched for, but the input still contains it. Tried out on Vivaldi (Chromium based browser) and Firefox, same results.

https://imgur.com/a/YpBKB0R

There is in fact no text in the searchTiddler. But there's a second Tiddler used to store the text input used for the keyboard driven input dropdown mechanism. You would need to delete also that Tiddler @bimlas...

[BurningTreeC]

@bimlas looking at your gif, you'd also need to delete the Tiddler $:/temp/search

[BurningTreeC]

No worries @bimlas , better late than never

[BurningTreeC]

@bimlas in this case, I think the 5.1.22 behavior is a bug. The dropdown shows exactly 500 matches because both the title and the all matches are limited to 250

[saqimtiaz]

@BurningTreeC should we perhaps trim[] the entered text before searching and checking minlength? Or at least a leading trim?

Edit: the idea being not to trigger a search upon only entering spaces. Or is there some use case where that behaviour is helpful?

[bimlas]

@bimlas in this case, I think the 5.1.22 behavior is a bug. The dropdown shows exactly 500 matches because both the title and the all matches are limited to 250

I think you’re half right, but then we should change the text next to the counter because it’s not clear: 500 matches -> 500 listed

Or is there some use case where that behaviour is helpful?

To quickly find out the number of existing, non-system tiddlers. :)

[Jermolene]

I think it might be prudent to not release 5.1.23 until after the weekend. There will be users that are unable to test during the working week, and there are still small issues being reported.

I'm hoping we might be ready very early next week, but let's see how things go over the weekend.

[saqimtiaz]

#5193 should be considered for merging. Tweaks the new reduce[] operator replacing the suffix with an optional second parameter to set the initial value.

[BurningTreeC]

@bimlas in this case, I think the 5.1.22 behavior is a bug. The dropdown shows exactly 500 matches because both the title and the all matches are limited to 250

I think you’re half right, but then we should change the text next to the counter because it’s not clear: 500 matches -> 500 listed

Yes I'm just half right, sorry it was just my first thought. My second is that I'll change it to show all matches, not just the listed ones

Or is there some use case where that behaviour is helpful?

To quickly find out the number of existing, non-system tiddlers. :)

[BurningTreeC]

I think it might be prudent to not release 5.1.23 until after the weekend. There will be users that are unable to test during the working week, and there are still small issues being reported.

I'm hoping we might be ready very early next week, but let's see how things go over the weekend.

@Jermolene I have three things on my list which I will tackle these days: the tags-dropdown-speedup, the codemirror brackets and selection highlighting and the counter on the search results list. Probably a fourth one in documenting the new behavior of the search dropdown

[BurningTreeC]

@bimlas , what do you think is the right behavior for the count - matches Button : showing the count of all matches with or without de-duplication? By that I mean - should dropdown items that are shown twice be counted twice or just once?

[Jermolene]

@Jermolene I have three things on my list which I will tackle these days: the tags-dropdown-speedup, the codemirror brackets and selection highlighting and the counter on the search results list. Probably a fourth one in documenting the new behavior of the search dropdown

Great, thanks @BurningTreeC

[Jermolene]

Today has yielded two new PRs with optimisations that could perhaps be merged for v5.1.23:

• Cache result of parsing macro calls which don't use text subsitution #5205 for caching the results of parsing macro calls
• use a linked list for filter runs. #5206 use a linked list instead of an array for filter runs. Part of the case for including this in v5.1.23 is that it introduces changes to the new filterrunprefix mechanism which might be hard to do later

My own opinion is that we have yet to see sufficient evidence that v5.1.23 has been tested with popular adaptations (for example, there was some discussion on the mailing list that Stroll has not yet been updated to make it compatible). So, I think there is a window of a day or so where we could merge those two PRs, and then release in a weeks time.

[saqimtiaz]

@Jermolene I am in favour of allowing an extra week for testing and merging these two PRs, but with the addendum that we have a strict moratorium on merging PRs apart from bug fixes, so as to have a stable candidate for testing.

[flibbles]

Yeah, @saqimtiaz, really sorry to be this late in the game with the #5206 change. But man would it be sad if backward compatibility prevented us from ever getting it at all.

[saqimtiaz]

@flibbles no need for apologies, it is an excellent optimization.

The comment about the moratorium on merging new PRs is mostly aimed at myself and @BurningTreeC as I am sure we both have stuff we would be tempted to get in there given the opportunity.

[BurningTreeC]

The comment about the moratorium on merging new PRs is mostly aimed at myself and @BurningTreeC as I am sure we both have stuff we would be tempted to get in there given the opportunity.

Amen.

[rmunn]

#5246 should, IMHO, be a release blocker, because the different behavior between the reduce operator and the :reduce filter run prefix is going to trip someone up. Right now is the time to fix it, so that we don't end up having to break backwards compatibility if we fix it post-release.

[danielo515]

Is there a list of known breaking changes?

[Jermolene]

#5246 should, IMHO, be a release blocker, because the different behavior between the reduce operator and the :reduce filter run prefix is going to trip someone up. Right now is the time to fix it, so that we don't end up having to break backwards compatibility if we fix it post-release.

Yes, we definitely need to resolve that now.

[Jermolene]

Is there a list of known breaking changes?

As per our usual policy, we have endeavoured to maintain backwards compatibility with this release. The vast majority of changes in a new release are additions (more filters, widgets, macros etc) which generally don't have a broad impact. The exceptions are:

• most bug fixes necessarily break backwards compatibility, and therefore could theoretically impact a plugin
• when we re-engineer a subsystem with significantly improved functionality we often have to break low-level backwards compatibiltiy. That happened in v5.1.22 with the new syncer, and to a certain extent in v5.1.23 with the new keyboard shortcuts, some of which have necessitated backwards incompatible changes

This time around, I don't think there is anything like the syncer changes in terms of backwards compatibility.

[saqimtiaz]

@Jermolene there is a small bug with the recent LinkedList change to filter run prefixes that needs resolving before release. Quick fix but needs to be done before release. I'll try to attend to it either late tonight or tomorrow morning.

https://github.com/Jermolene/TiddlyWiki5/blob/master/core/modules/filters.js#L300 expects an array but now receives a linked list.

Edit: needs to be
results.clear() instead of results.splice

[flibbles]

Ooh. This is my bad. I can make a PR for it now actually. I throw in a test too to make sure this travesty never happens again.

[saqimtiaz]

@flibbles that would be great and no worries, it's a hole in our testing coverage.

[flibbles]

Fix! #5251

[rmunn]

Might be good to merge #5233 in before 5.1.23. It just updates the Modified field on a tiddler that has a "New in 5.1.23" section, but whose Modified field shows a date in 2014.

[Jermolene]

Thanks everyone. I'm hoping now that we might release over the weekend, perhaps Sunday 20th. There are a few outstanding issues/PRs from the last couple of 48 hours or so that I need to review, and I need to update the release note.

[saqimtiaz]

Thank you, the collaboration that went into this release has been a real pleasure.