Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce distinct "continued" char for broken line "non-branches" #64

Closed
wants to merge 8 commits into from

Conversation

jdtsmith
Copy link
Contributor

@jdtsmith jdtsmith commented Mar 13, 2023

When vundo runs out of a room when displaying a linear chain of nodes, it must move down and fill the available space on the next line, "swerving" the chain down using a last-branch char. This can make it hard to visually parse complex graphs. Since key bindings to get to the next node along the chain vs. next sibling are different this can also make it tougher to navigate.

This PR adds a distinct continued character to make spotting such non-branch "bent lines" easier. Right now it uses the curved "arc" version for unicode and a single ' for ascii.

Update: see below for the the current improved unicode idea: heavy lines connect single continuous main branches.

Note: we could make this an alternative glyph set, such as vundo-unicode-symbols-fancy.

When vundo runs out of a room for a linear chain of nodes, it must
move down and fill the available space, breaking the chain with a
lst-branch char.  This adds a distinct character to make spotting such
non-branch broken lines easier.
@jdtsmith
Copy link
Contributor Author

jdtsmith commented Mar 13, 2023

Added a commit with the double line glyphs, which I think looks better.

@jdtsmith
Copy link
Contributor Author

jdtsmith commented Mar 14, 2023

Found another rare corner case: if the node where the line "bend" occurs has children (as in the selected node below), we need a continued-branch too. I tried the unicode single/double heavy flavor for this (which are the only ones with all the necessary glyphs); best overall I think (see below for yet better):

[snip]

(to actually see the branch glyph, you also need #63).

@jdtsmith
Copy link
Contributor Author

jdtsmith commented Mar 14, 2023

On further thought, this makes me wonder if we should just always use heavy connectors for the "main branch", not just on the swerving/broken line part.

Here's how that looks for a particularly tortuous tree structure:

image

I think this is pretty nice. You can see at a glance how long the main lead of a single branch is, and where a and e will take you, no matter how "broken" it becomes. And it allows better key recall:

  • b/f to follow the bold branch
  • n/p to hop among their roots

@casouri
Copy link
Owner

casouri commented May 3, 2023

I personally don't think this is very necessary, since if you think about it, in real usage settings, we very rarely have complicated trees like this. More often than not, the tree is a simple single branch with occasional branching here an there.

@ideasman42
Copy link
Contributor

Personally I prefer the current display too - having more straight lines makes it visually less "noisy", although I can see in some cases it saves some room.

It could be an option I suppose?

@jdtsmith
Copy link
Contributor Author

jdtsmith commented May 4, 2023

@casouri It's indeed rare to get these "broken/bent" branches, but I find them very confusing when they do appear.

@ideasman42 this isn't about whether to keep branch lines straight or not. That's obviously the first preference, but vundo has no choice but to bend a single branch line downward when it would run into another branch. They can't pass through each other! This PR is about how to visually display this unavoidable (but relatively rare) branch break/bend.

@casouri
Copy link
Owner

casouri commented May 7, 2023

My problem with the proposed method is that it is not immediately clear what does the highlight mean. If someone happens to have a more complex tree, and sees this highlighted stem, how could they know it's supposed to mean that the edge is a normal stem?

If we really want to make it clear, I would highlight the branching stems instead. It's not very hard to understand (without explanation) that highlighted stem = can switch between, unhighlighted stem = can't switch.

@jdtsmith
Copy link
Contributor Author

jdtsmith commented May 8, 2023

Not sure I understand your point, but maybe that's because I'm not sure what you mean by edge.

Put simply, if there is no highlight, you cannot tell at a glance whether the relationship between two adjacent nodes is i) parent->1st child or ii) parent-> other sibling. Type ii) always involves a lateral dislocation (i.e. using a branch char). Confusingly, sometimes parent->1st child also involves a branch character, to juke out of the way. The proposed method simply highlights all parent-1st child relationships, i.e. the ones you can navigate along purely with left/right.

I agree it's pretty rare to have such complex and "bunched up" trees though. I've been checking "real files" and it's maybe once a week I encounter this. Maybe if you save your undo list and go back over it a bunch, complexity would build up (I don't).

@casouri
Copy link
Owner

casouri commented May 8, 2023

Ah, sorry, I could be more clear. I like the second proposal better, as it's more consistent. But allow my to propose a third way: for each parent, if it has more than one child, highlight all the edges between that parent and its child. I feel like it's more understandable and clear-to-see that way.

I'm still not sure if the whole highlight thing is necessary tho.

@jdtsmith
Copy link
Contributor Author

jdtsmith commented May 8, 2023

That's a very interesting idea.

I merged master and implemented your proposal No. 3: highlight parent->child when child has siblings. So all "forks" with two or more tines get highlighted.

Looks like this (without compact display):

image

and with vundo-compact-display=t:

image

Use:

(progn (random "HIGHLIGHT-SIBLING-EDGES") (vundo-stress-test 100 30 10 nil))

to reproduce this tree.

BTW, when navigating and saving even fairly small but relatively "branchy" trees like this one, the "lost timestamp" issue (#66) is showing up in force — green circles disappear quite often (give a try).

@casouri
Copy link
Owner

casouri commented May 8, 2023

Looks pretty good, I like that it improves two things: now it's clearer where you can switch branches too. I noticed that you implemented it by adding new characters, is it possible to use faces instead? Like a bold face? Also there's some whitespaces changed to tabs in the commit, make sure there are only spaces in the file.

@jdtsmith
Copy link
Contributor Author

jdtsmith commented May 9, 2023

Nice suggestion. Face is possible now, though it wasn't before since you needed the character (half heavy). Having no extra chars is simpler. Unfortunately, bold doesn't seem to be too visible for the horizontal stem for me, but a color could always be added to the face, like:

image

or color without bold:

image

The WS changes you see are undoing some tab-indenting that apparently snuck in.

@casouri
Copy link
Owner

casouri commented May 9, 2023

Cool cool. Could you squash the commits?

@jdtsmith
Copy link
Contributor Author

jdtsmith commented May 9, 2023

Because I merged master in the meantime, squashing leads to conflicts. I squash merged to a new branch and opened a different PR with a more appropriate name; see #71.

@jdtsmith jdtsmith closed this May 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants