Add a comparison table when existing tiddler is about to be imported

[yaisog]

This PR adds a table that compares the date and size (in lines) of the existing and incoming tiddler in those cases where an incoming tiddler already exists. It also gives a verbal indication which tiddler is older, newer, smaller, larger.

If the incoming tiddler is older, a red border is additionally drawn.

Closes #7211

[vercel]

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

Name	Status	Preview	Updated
tiddlywiki5	✅ Ready (Inspect)	Visit Preview	Feb 28, 2023 at 4:09PM (UTC)

[saqimtiaz]

I would strongly suggest that we explicitly solicit end user feedback for these changes over the course of a release cycle before making any changes.

We have previously worked on making the import related messages more concise in light of user feedback that they were too long and this was diminishing their usefulness. We should also consider whether redesigning the import UI is a better way to proceed rather than shoehorning more information into the already cluttered existing UI.

[Jermolene]

Thanks @saqimtiaz

For reference, here's a screenshot of the message when importing an older version of a tiddler:

[pmario]

hmmm. Now that I see it, I would like to make it much less intrusive and easier to manipulate. ..

What if we create a tiddler eg: $:/core/ui/ImportListing/Overwrite/xxxx and transclude it into the overWriteWarning() macro using a list

Edit: Needed to update the variables below

<$list filter="[subfilter<payloadTitleFilter>is[tiddler]]" variable="void">
	<$list filter="[all[shadows+tiddlers]tag[$:/tags/ImportInfo]]" variable=verbose>
		<$transclude tiddler=<<verbose>> mode=block/>
	</$list>
</$list>

So it will be possible, to add additional info to the overwrite text on demand.

[pmario]

It's exactly the same, so the info is redundant and way to much.

[pmario]

So as Tony wrote. If there is a verbose detection it should be much more verbose. Listing exactly, what has changed. Otherwise it does not contain new info.

[yaisog]

So as Tony wrote. If there is a verbose detection it should be much more verbose. Listing exactly, what has changed.

Tony's is a different use case. I'm trying to sync two wikis manually, by import. I'll be importing a bunch of tiddlers and need an at-a-glance check if I'm about to do something stupid. No? Fine. Go import.

Otherwise it does not contain new info.

I think there's a lot of prairie between "A tiddler with this title already exists." and the detailed output of the diff tool that is already part of the import dialog. I could imagine a plethora of granularities of data provided which would fit between these two extremes, each containing successively more information than the one before.

Yes, the layout can be optimized, the CSS is just a rough draft. Let's not get too hung up on that. At this point it is more about function than about form. The columns could be drawn above-and-below for narrower screens. I obviously designed it on a larger screen, where it looks fine:

And also yes, if the files are identical, it's a good idea to replace the table with a corresponding note.

I also support the idea of making it configurable and extensible via tagged tiddlers. Then, it might be shipped with at least the original (default) and verbose versions, and a dropdown might be added above the status column that lets users select their favorite choice.

[yaisog]

If the utility of this PR is not self-evident, I think I'm wasting my time trying to convince people.

Mario's idea should be another PR for another day.

[Jermolene]

If the utility of this PR is not self-evident, I think I'm wasting my time trying to convince people.

Mario's idea should be another PR for another day.

Perhaps it's worth having a discussion on talk.tiddlywiki.org about the options. There has been a fair amount of discussion about the underlying problem and so I'd still be interested in exploring whether there are some simple improvements.

[yaisog]

I will post my code for those who need this enough to be willing to overwrite a core tiddler, like I did. In that context we can maybe get some more opinions. To me this functionality – akin to probably every file manager out there – is so basic that I don't really see the point of discussing this further.

[Jermolene]

I will post my code for those who need this enough to be willing to overwrite a core tiddler, like I did. In that context we can maybe get some more opinions. To me this functionality – akin to probably every file manager out there – is so basic that I don't really see the point of discussing this further.

The issue here is the perennial one that we want to ensure that any updates to the core are the best we can do. Sometimes that does risk making "perfect be the enemy of good", of course.

[saqimtiaz]

Yes, the layout can be optimized, the CSS is just a rough draft. Let's not get too hung up on that. At this point it is more about function than about form. The columns could be drawn above-and-below for narrower screens. I obviously designed it on a larger screen, where it looks fine:

If this indeed an early draft of the implementation, this should have been created as a draft PR with a note that is it meant to facilitate discussion and it would then have been reviewed differently. PRs for core changes should be self-sufficient, that is there should not be an intent that a second PR will be required to fix a first implementation to address usability issues that are already evident.

PRs are for concrete implementation that the author considers appropriate to be merged into the core as is, unless otherwise specified, and are reviewed as such.

To me this functionality – akin to probably every file manager out there – is so basic that I don't really see the point of discussing this further.

I find this a very counter productive way of looking at things. No one has questioned the underlying need, or the proposed idea for improved functionality, however the implementation definitely needs discussion, user feedback and further improvement before it can be considered for the core.

[yaisog]

this should have been created as a draft PR

I am sorry, but I am not yet as well versed with the intricacies of GitHub as I maybe should be. The draft status mainly concerns the CSS. I'm afraid if finely polished CSS is required to submit a PR, you will see no further ones from me.

No one has questioned the underlying need [...]

I am again sorry, but I experienced this differently. I felt nothing but blowback from the minute I opened issue #7211, also and especially pertaining to the fundamental necessity of the proposed feature. This was draining too much of my time and energy, which eventually led me to close this PR.

[pmario]

If the utility of this PR is not self-evident, I think I'm wasting my time trying to convince people.

Mario's idea should be another PR for another day.

I'm not against it. Just seeing it implemented I don't like it anymore. At the other thread #7211 I thought, that we would need additional help from JS code. So I did no explore it further. ... Your experiments / PR here shows that there is much more usable info in the import tiddler as I expected.

My experiments with https://talk.tiddlywiki.org/t/howto-compare-the-content-of-a-shadow-tiddler-with-the-content-of-an-overwritten-shadow/6191 also helped me to get a better understand of the possibilities diff-match-patch gives us.

Having a look at the new images taken by Jeremy and myself, I just think that if there is new visual overhead it needs to provide a lot more new information.

1. We do have the info that "A tiddler with that title already exists"
2. Then there is a table with almost the same info with the date
3. Then there is some more info "1 lines" that tells us again that there is a difference of 1 line ... But nothing about the real differences.

IMO that should be improved in the way that #3 has the biggest potential to contain useful info, which should be easy to access for the user.

---

I think I'm wasting my time trying to convince people.

I can see your frustration, but I can ensure you, you are not wasting time. In the contrary, it's inspiring that you keep pushing.

I think this type of experiments are needed to get things going and even if this PR is not merged, I think there is a high chance, that in the end we will have a good solution, we all will be fine with.

just some thoughts.

[pmario]

yaisog wrote

I will post my code for those who need this enough to be willing to overwrite a core tiddler, like I did. In that context we can maybe get some more opinions.

Users do not like to overwrite core tiddlers, because that's usually problematic with updating. .. That's the whole reason why we did create the "cascade" filter stuff.

With my suggestions posted at: #7306 (comment) I think it would be possible to keep the things backwards compatible and also have the possibility to create experiments and plugins that can show more info. Even overwriting the core 1 line message, if needed.

In the light of your extension this 1-liner imo is redundant and having the possibility to remove it, will reduce visual complexity.

[saqimtiaz]

I'm afraid if finely polished CSS is required to submit a PR, you will see no further ones from me.

From my perspective a PR needs to be polished enough in all aspects before it can be merged, so that a holistic judgement can be made as to whether it is a good implementation. That does not however mean that the original PR author need do it entirely by themselves. Through discussion and collaboration others can contribute and help get a PR ready for merging, though this requires a certain degree of patience.

[pmario]

I did a bit more experiments. I do find something like this much more convenient, because the info is easier to understand.
At least for me.

I'll create a PR for this one. It will not contain the verbose elements. Just the backwards compatible but more hackable stuff.

The verbose info may land in my bundler plugin in the future.

[Jermolene]

I am again sorry, but I experienced this differently. I felt nothing but blowback from the minute I opened issue #7211, also and especially pertaining to the fundamental necessity of the proposed feature. This was draining too much of my time and energy, which eventually led me to close this PR.

I am sad to hear that. Reading the thread back, perhaps my terse comment implied disapproval too, which was not my intent.

As @saqimtiaz points out, it can be frustrating dealing with incomplete PRs (here's an example of me talking about it), but that has to be balanced with our desire to help all contributors (whether newcomers or not) to navigate the complexities of the process. Our aim is for the community to be supportive and understanding.

Happily, you've already shown considerable success in getting your work into the core, and I hope you'll be able to continue to do so.

[pmario]

I did one more experiment. ... It clearly shows that there is still room for improvement. For me it's extremely important, that I can import state- and temp-tiddlers. ...

But as shown in the image below with a realistic import tiddler with existing, blocked system, blocked plugin and a "user selected" state tiddler. .. There is a problem with the $:/temp/info-plugin .. It does not have a created or modified info. ...

As you can see, there is a lot going on. Especially with redundant info that only makes interpreting the info more difficult.

There is only 1 info box, that actually contains "relevant" info, where the existing is "newer" than importing. IMO only this info should stand out. The rest should not be there at all

just more thoughts.