A peculiar PR request

#1

I would post this PR myself if I could, but my abilities are limited to doc PRs so, alas, I’m hoping someone can help and post this as a PR:

The tiddler $:/core/ui/Buttons/delete ends with a redundant empty line.
This can be compared to e.g $:/core/ui/Buttons/close which does not end with an empty line.

Could someone please make a PR with that redundant empty line removed?

Yeah… an odd request, here’s the background:
I’m creating a plugin for a very useful feature that I’ve been using every day for years. I want to share it with the community but for it to work as a plugin, that empty line mustn’t be there. Basically, the plugin will rely on doing an overwrite of the delete button code via the following trick but the define pragma cannot have an empty line below </$button> (and even if it could, it would still not be optimal for other reasons):

\define remove()
</$button>
\end

<$let newtext={{{ [[$:/core/ui/Buttons/delete]get[text]removesuffix<remove>addsuffix<insert>] }}} > ...

Thank you!

#2

I’m curious what limitations you have if you already know the line in the source code and you can make PRs. I thought docs and core were one and the same repository.

I’m also curious why TW is affected by a new line. One would think (IMHO) that white space would not have such far reaching side effects.

Let me know if you are still stuck; but I think—as described—this should be an approachable PR to accomplish even via the GitHub web interface; and I would not wish for you to loose out on any attribution.

#3

In my tests, I could get the result I think you want just by adding a line return inside the remove() macro. I changed <insert> to [insert] for the test (because I don’t know what <insert>

refers to), and got:

image

Lots of parts of TW are sensitive to white space. It’s one of those things you have to keep in the back of your mind when you don’t get the results you were expecting. But in this case, Mat wants the white space (linefeed) removed so that it will match his removesuffix construct.

1 Like
#4

@twMat I appreciate your idea and frustrations and followed your very useful feature link, and see how it confuses some. The last thing you said was;

Would it be possible to add an action param to the close and delete buttons, to allow for actions defined elsewhere? I’ve had to schlepp around my crude core overwrites outlined here above for several years now.

  • Close yes, delete no.

Did you see my https://all-button-actions.tiddlyhost.com/ which is a suggested approach to adding this hackability, ultimately to the core?

  • It’s trivial with this to add actions to any button (except 1 delete) - eg close is easy.
  • This is why the only non-trivial button is the delete button
    • This is because $:/core/ui/Buttons/delete its actions use the actions=<<cancel-delete-tiddler-actions "delete">> macro, and this is defined in more than one place in template tiddlers, so you must verify the delete. See the Issues section on the Home tiddler.

It is always uncomfortable when you want to implement one idea, and a few other items could/should be done before hand to make implementing your idea simple.

Perhaps a way around this is create an alternative close and delete button, that looks like the existing ones, but does what you want them to. Hide the standard ones behind more and show yours on the toolbar.

  • Note no PR needed or even core tiddler override, just alternate buttons to achieve what you want.

I can do “documentation PR’s for tiddlywiki.com”, but I am little rusty on PR’s for core tiddlers.

1 Like
#5

Not sure how to describe it but there is a difference in how doc tids vs non-doc tids are set up on gh. Making doc PRs is only somewhat confusing but making non-doc PRs is a whole different matter.

Thanks but you’d be doing me a favour!

#6

Are you sure? Here is the tiddler in question, start editing and make a PR and just let us know if you run into any issues:

#7

@Mark_S and @TW_Tones here’s the code. So the idea is to navigate back to the tiddler you came from when you either close or delete the current.

If you try it out, you’ll see the “close” works but not the “delete” - because of the empty line at end of delete:

\define remove()
</$button>
\end
\define insert()
<$action-navigate $to={{{ [list[$:/StoryList!!list]before<currentTiddler>] }}} />
</$button>
\end

$:/core/ui/Buttons/delete
$:/core/ui/Buttons/close

<$let subfilter='[get[text]removesuffix<remove>addsuffix<insert>]' >
<$button>
<$action-setfield 
   $tiddler="$:/core/ui/Buttons/delete"
   text={{{ [[$:/core/ui/Buttons/delete]subfilter<subfilter>] }}} />
<$action-setfield 
   $tiddler="$:/core/ui/Buttons/close" 
   text={{{ [[$:/core/ui/Buttons/close]subfilter<subfilter>] }}} />
X
</$button>
</$let>
1 Like
Filter problem: "then...else" but carrying the input
#8

@saqimtiaz and @sukima - thank you! Hm, I guess I was wrong about the level of difficulty in editing code tids vs doc tids (…wasn’t there a big difference previously? I’m almost sure there was?)

But one issue remains in that How the heck do you locate the tiddler in the gh structure? For doc tids, there is the pink ribbon on tw .com but why isn’t there the same for code tids?

I just posted this PR :slight_smile:

#9

The GH directory structure is very intuitive if you spend a few minutes to think about it once.

It would require that someone put in a rather significant amount of work to get that into place, with no tangible and assured pay off.

#10

Never, ever rely on an empty line at the end of anything. The next time someone will handle that tiddler it may have an empty line again.

Some IDEs have a default setting that every file should end with an empty line. … Some IDEs don’t. In reality it shouldn’t matter

#11

Hm… I can give examples for how to get lost there but on the whole it is - of course - just a lot simpler with a direct link (as via the pink ribbon). But so is that how core contributors do it - they manually navigate down in the gh repository tree to locate the tiddler every time?

I’ve kind of assumed the lacking pink ribbon link for code tids was a way to prevent just anyone from making code PR’s - not that it was more difficult to implement such a link. Wouldn’t it be pretty much the same mechanism as for the doc tids (…what’s the difference? Both are links to a location in the gh tree? Or maybe the docs tids are structured in the gh tree so they’re more easily mapped?)

#12

Also worth noting that the POSIX standard actually requires a line break at the end of each line for it to be considered a valid line.

A sequence of zero or more non- characters plus a terminating character.

from: Definitions

#13

Hm, that sounds like a lack in how we handle code contributions then, allowing arbitrary empty lines to be inserted. As Mark noted above, there are quite a few cases where empty rows affect the outcome in TW.

Do you have a suggestion for how I can avoid this problem in the code I posted here above?

#14

I suspect most core contributors use TiddlyWiki on node.js and either edit via TiddlyWiki, or directly in a code editor. As I mentioned, the structure is very intuitive if you have spent a few minutes with it once.

TiddlyWiki on node.js tracks where it loads content tidders from, but shadow tiddlers within plugins are different and there is no mechanism to trace where the component tiddler of a plugin was loaded from.

#15

OK, but then we have a bigger issue because the code is inconsistent with terminating line breaks, compare the mentioned delete and close buttons.

#16

Adding to it, @twMat you can search for a file in Github by using the file finder. Press t and it will open the file finder mode for you.

Another way is to search for a string in the repository. But when you open the file from the result, you must remember to switch to the HEAD version of the file.

Finally, on desktop you can use commands like fd, find, rg or grep to search for files.

2 Likes
#17

You are right. But that is one more argument, that no plugin should rely on a consistent behaviour of the core about whitespace. … Except if there is the \whitespace trim pragma.

There is a lot of whitespace in the core UI which creates redundant P tags. So there are a lot of redundant HTML paragraphs in the core UI, which make styling and theme development problematic.

I want to get rid of them. They are one of my biggest personal pain-points in TW. It is already extremely hard to get rid of them because of Jeremy’s backwards compatibility concerns.

This is also one reason, why I do try to add indentation to the core UI, for human readability and it will define whitespace trim, which introduces the need to define the spacing between HTML elements using CSS and not spaces. …

1 Like
#18

hmm, Back to the code at the OP. … What you want to do here is called monkey patching, which has on inherit and fatal flaw. It 100% relies on the original code, to be always the same.

So in reality not only the trailing whitespace is already a problem for your code, it also relies on </$button> be the last word of that code. There is a good chance, that this assumption is right, for something named: $:/core/ui/Buttons/xxx, if there wouldn’t be Murphy’s Law

“Anything that can go wrong will go wrong, and at the worst possible time.”

I can only suggest: Search for a method to solve the problem in a solid way.

1 Like
#19

Great post. Very informative about some “backroom essentials”

Yeah!! Right-on broheim.

  • The <p> pollution is likely the ONLY thing I dislike in TW.

I guess it is there because @jeremyruston, at creation time, could not foresee the issues on basic layout control it has led to?

This is the ONLY issue where I think TW (@jeremyruston’s concern) should NOT be backwards compatible. Because it it is SILLY not to move on to a cleaner thing.

A comment from me
TT

#20

Not sure how… but here’s an idea - what do you think about it:

In all(?) core buttons, insert an action param and give it an argument that the user can define. For example:

title: $:/core/ui/Buttons/close
text:
<$button actions="{{$:/core/ui/Buttons/close-actions}}">

$:/core/ui/Buttons/close-actions is empty (non-existing) by default but users can then define arbitrary strings of actions.

With that said, I did request this but there was no interest or feedback.