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

Add proper "jump back" command #190

Open
pokey opened this issue Aug 6, 2021 · 7 comments
Open

Add proper "jump back" command #190

pokey opened this issue Aug 6, 2021 · 7 comments
Labels
enhancement New feature or request help wanted Extra attention is needed
Milestone

Comments

@pokey
Copy link
Member

pokey commented Aug 6, 2021

Every time you issue "pre", "post", "take", or "clear", it will capture previous cursor location and keep it up to date

Can use "bounce" or just "back" for this one

Would also want to support "bring air back", eg without "to"

@tararoys
Copy link

I vote for this!

@AndrewDant
Copy link
Collaborator

To be clear this would be a special target?

@pokey
Copy link
Member Author

pokey commented May 6, 2022

It would be a special mark. Will be best to do as part of #191

@pokey
Copy link
Member Author

pokey commented May 9, 2022

Ok actually I take it back. No need to go super general. I'd just get something working for a start.

I'd start by copying https://github.com/cursorless-dev/cursorless/blob/main/src/core/IndividualHatMap.ts

The important thing is to register your ranges with the range updater to ensure that they stay up to date as the document changes. See https://github.com/cursorless-dev/cursorless/blob/main/src/core/IndividualHatMap.ts#L30 to see where it is called there, and also the docs

@pokey
Copy link
Member Author

pokey commented May 10, 2022

Some things to keep in mind:

  • Should make this thing a graph component. Can grep for editStyles to see a good example. You may need to give it an init function and call it from extension.ts as well
  • The class should register to listen to all selection changes in order to push them on a stack
  • Prob want stack to have max len
  • It should have a method to pause its listening, which will be used by CommandAction to avoid polluting stack during hacked commands that do a jump
  • The stack should be accessible from command context during command execution so that the top (and potentially below) can be accessed as a mark
  • How do we pop the stack? We somehow would need to detect that we're doing a "take back" command, but currently "take" and "back" would be orthogonal so neither knows it's a "take back". Maybe we could have a special command just called "back" that knows to pop the stack

@Will-Sommers
Copy link
Collaborator

Heyo @pokey, here's a few notes/questions

Prob want stack to have max len

It should have a method to pause its listening, which will be used by CommandAction to avoid polluting stack during hacked commands that do a jump

❓I'm curious about this one, the description implies that we should use this for just a subset of actions. What in your mind are hacked actions?

The stack should be accessible from command context during command execution so that the top (and potentially below) can be accessed as a mark

How do we pop the stack? We somehow would need to detect that we're doing a "take back" command, but currently "take" and "back" would be orthogonal so neither knows it's a "take back". Maybe we could have a special command just called "back" that knows to pop the stack

✅ / ❓ Makes sense to me. Will we have to check DFA/public on this one?
❓ One thought I have about this is that it might make sense to keep two stacks to allow for movement forwards and backwards, similar to a "undo/redo".
❓Will "back" only pair with certain actions?


Some edge cases to think about:

  • What happens when the code underlying the selection is deleted? Should we keep track of the node ids somewhere and proactively prune the stack? If we go this path, how to we represent that the underlying file has changed to the user? What is the correct behavior?
    • I think we should proactively prune and just tell the user that the previous node is gone and so the command is ambiguous. I don't know if we have a better way to communicate apart from the pop-up in the bottom right.
  • This isn't an undo/redo command level action so we don't need to think about storing the underlying text, just the selection.

@pokey
Copy link
Member Author

pokey commented May 11, 2022

It should have a method to pause its listening, which will be used by CommandAction to avoid polluting stack during hacked commands that do a jump

❓I'm curious about this one, the description implies that we should use this for just a subset of actions. What in your mind are hacked actions?

What is "this" here? "back" will just be a mark which refers to the previous cursor location, so can be used for any action. The stack of previous cursor locations will be populated by listening for any changes to the selection, no matter where they come from. However, there are certain cases we want to avoid things ending up on the cursor location stack, because the cursor movement is transient. These examples are the "hacked" actions I mentioned, where we move the cursor because there is no properly cursorless way of doing them. For example, today the "copy" action is implemented by moving the cursor, copying, then moving the cursor back. For this case, it is confusing if the location that the cursor moved is on the previous location stack, because from the user's perspective the cursor never moved

make sense?

How do we pop the stack? We somehow would need to detect that we're doing a "take back" command, but currently "take" and "back" would be orthogonal so neither knows it's a "take back". Maybe we could have a special command just called "back" that knows to pop the stack

✅ / ❓ Makes sense to me. Will we have to check DFA/public on this one?

Should have no DFA impact because it's an independent command

❓ One thought I have about this is that it might make sense to keep two stacks to allow for movement forwards and backwards, similar to a "undo/redo".

Two stacks is an interesting idea.

❓Will "back" only pair with certain actions?

So there will be two things: "back" the mark and "back" the command. We might call them two different things tbh. Maybe "last" for the mark, and "back" for the command. The mark can pair with anything because it's just a mark, the command is just a standalone command that moves cursor and pops stack so doesn't really pair with anything

"

Some edge cases to think about:

  • What happens when the code underlying the selection is deleted? Should we keep track of the node ids somewhere and proactively prune the stack? If we go this path, how to we represent that the underlying file has changed to the user? What is the correct behavior?

    • I think we should proactively prune and just tell the user that the previous node is gone and so the command is ambiguous. I don't know if we have a better way to communicate apart from the pop-up in the bottom right.

I'd just let the range updater handle this one for now. If it becomes annoying we can figure out removing from the stack

  • This isn't an undo/redo command level action so we don't need to think about storing the underlying text, just the selection.

Yeah we just keep the range, and let the range updater keep it up to date as the document evolves

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants