search

TiddlyWiki / TiddlyWiki5

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

[BUG] 5.1.22 upgrade forgets default tags in journal entries #4721

Closed tfeb closed 3 years ago

tfeb commented 4 years ago

I use TW as a diary among other things (for which it is wonderful), and I have it set up so that the new journal entries get a log tag by default. This is done simply by setting the default tags in the control panel.

I recently upgraded from 5.1.21 to 5.1.22, using what I think is the standard upgrade process:

  1. make a backup;
  2. open a downloaded copy of the upgrade TW;
  3. point it at my main TW;
  4. let it upgrade it;
  5. save things and copy to the right place;
  6. profit.

This went fine, until today I noticed that my new journal entries did not get the default log tag. And this is because the control panel no longer had it set as a default tag for them. Putting it back in the control panel has resolved the problem.

This is not a major problem since it is easy to fix, but I think the upgrader should not have forgotten configuration like this. It never has before (my current TW originated at 5.1.9 and has been upgraded through most versions since then).

jeremyredhead commented 4 years ago

This is due to a change in how/where default tags are saved from 5.1.21 to 5.1.22 (and then back again in upcoming 5.1.23)

Previously the default tags were stored in the text field of $:/config/NewJournal/Tags, but 5.1.22 used the tags field instead, so the tag picker UI could be used. But this was recently reverted by #4600 due to bug #4591 (the config tiddlers were showing up as being tagged with the default tags... because they were :P)

That's definitely annoying that the upgrader didn't account for that, but it seems like it should be fixable (I say "seems" because I'm not familiar with how exactly the upgrader works)

tfeb commented 4 years ago

Probably the important thing is that the 5.1.23 upgrader copes with TWs that are at 5.1.22: it looks as if it will automagically cope with upgrades from TWs at versions older than that.

Jermolene commented 4 years ago

Thanks @tfeb @jeremyredhead that makes sense. It sounds like the upgrader should detect $:/config/NewJournal/Tags having tags but no text, and move the tags into the text field if so. Is that right @BurningTreeC?

BurningTreeC commented 4 years ago

Yes @Jermolene , sorry for this confusion I introduced in 5.1.22

I don't know if the upgrader should detect those things or if we should leave it as it is with the changes reverted in 5.1.23

saqimtiaz commented 3 years ago

bumping this as we get closer to 5.1.23 @Jermolene @BurningTreeC, so that the upgrade experience from 5.1.22 is as smooth as possible.

BurningTreeC commented 3 years ago

Hi @saqimtiaz , this issue should be resolved

BurningTreeC commented 3 years ago

@saqimtiaz ah yes, we can do something

Jermolene commented 3 years ago

Thanks @saqimtiaz @BurningTreeC do you intend to proceed with the upgrader?

BurningTreeC commented 3 years ago

@Jermolene I thought the fix was using both the text and the tags fields for the tags of new tiddlers. But if the upgrader can fix this problem, it would be better to do so

Jermolene commented 3 years ago

I thought the fix was using both the text and the tags fields for the tags of new tiddlers. But if the upgrader can fix this problem, it would be better to do so

Apologies, I hadn't considered that using both fields might be an option. On balance I guess it keeps things simpler to handle it in the upgrader. One issue is that Node.js wikis cannot use upgraders, so perhaps we should err on the side of caution and do both?

BurningTreeC commented 3 years ago

@Jermolene, I don't know how to handle it in the upgrader, but I can prepare a PR for the wikitext stuff

Jermolene commented 3 years ago

Hi @BurningTreeC did you implement the code to use both the text and the tags fields for the tags of new tiddlers? I think it's the best solution for the moment. If not, could you do so for v5.1.23?

BurningTreeC commented 3 years ago

Hi @Jermolene, yes the code to use both the text and the tags fields for the tags of new tiddlers/journal tiddlers is implemented

saqimtiaz commented 3 years ago

@Jermolene that code is in place, see #5060.

The question is if we also want an upgrader module to resolve the problem as well, as you suggested earlier in this thread?

Jermolene commented 3 years ago

Cool, I'm inclined not to bother with the upgrader because it'll have limited impact given that it doesn't run for Node.js upgrades.

saqimtiaz commented 3 years ago

@Jermolene this is then resolved and can be closed, as well as #4871