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

Allow for jumping backwards. #28

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Conversation

lsponzoni
Copy link

Creates a jumpstack that allows for jumping backward until the jumpstack is empty.
Solving issue:
#27

Creates a jumpstack that allows for forward and backwards.
src/main.rs Outdated
@@ -137,6 +138,7 @@ impl Bk<'_> {
line: 0,
mark: HashMap::new(),
links: epub.links,
return_stack: Vec::with_capacity(5),
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't like this magic number

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. Should I change it to a constant, or should I change it to use just new Vec?
The idea behind 5 was a soft limit for most cases.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just new vec. there's no reason to try to optimize this

@aeosynth
Copy link
Owner

aeosynth commented Apr 25, 2023

looks good, but can you make it go forwards as well? shouldn't be much more code

in fact you could rewrite the ' mark to use this stack

@lsponzoni
Copy link
Author

I can, I can. Do you mean putting the other ' mark ocurrences as the stack?

@aeosynth
Copy link
Owner

it looks like you're not updating the ' mark when using the jump stack, that is not great behavior. opening vim it seems that their behavior isn't obvious to me either when using jump stack and the ' mark. we need to be clear on when it does and doesn't update. i'm not sure about the implementation, do whatever you like and i'll review.

@aeosynth
Copy link
Owner

my initial implementation thought was that ' could be a pointer to the stack, iow just the index number, but that might not be useful

This lets the jump stack be traversed as a double linked list
jump 1 <-> jump 2 <-> jump 3 <- Last jump

This is implemented by adding a int that points to the top of array.
stack_pointer
Array elements are always picked one less than the top.
The destination of the last jump destination is not in the stack. So jump forward can't really move to it.
But the jump n - 1 should point to the link that you press to make it.

When you click a link and stack_pointer is pointing to last
jump 1 <-> jump 2 <-> jump 3
                             *stack_pointer
jump 1 <-> jump 2 <-> jump 3 <-> jump 4
                                        *stack_pointer + 1

However, when clicking a link on a stack_pointer pointing before that:
jump 1 <-> jump 2 <-> jump 3 <-> jump 4
                   *stack_pointer

jump 1 <-> jump 2 <-> new jump 3
                                  *stack_pointer
Now jump forward can traverse back and forth from all positions, not
just where last link was.
a: link b
b: link c
c: link d
d: From here

Jump forward can go to b c d and backward go c b a.
This was a small change that jump back now saves where it is before
jumping back.
Almost all current use of '' could be remapped to backspace 1 time.
I've kept marks unrelated from jump list. And mark jumping is not part
of the jump list.

* jump_reset uses are changed to jump_back
* Almost all mark('\'') can be safely removed.
Change save_jump so that when the stack top is the same as the item
added, it does not push the jump.

Jump 1, Jump 1 ❌
Jump 1 ✓
@aeosynth
Copy link
Owner

aeosynth commented May 5, 2023

@lsponzoni how's it going?

@lsponzoni
Copy link
Author

I think as a feature it is complete. I could refactor the code.
What do you think of it?

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.

2 participants