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

[VS] Changes can’t be undone that we’re made by participants who left the session #1396

Closed
therealjawss opened this issue Dec 7, 2018 · 10 comments

Comments

@therealjawss
Copy link

therealjawss commented Dec 7, 2018

I see that there is a related issue that requested for local undo stacks #7 which makes sense when people are collaborating on different parts of the code.

However, in our testing scenario, my collaborator "accidentally" deleted everything, and disconnected. there was then no way for me to undo the delete once they were gone from the session.

not quite sure if this is a bug or a feature?

@lostintangent
Copy link
Member

@therealjawss Hey! Are you using VS or VS Code? I just ask because we recently made a change to VS Code to support a global undo stack.

@therealjawss
Copy link
Author

Using the Visual Studio 2019 preview, sorry should have provided more detail as the instructions said. >_<

@therealjawss therealjawss changed the title Undo changes made by collaborator who has disconnected due to interrupted connection [VS] Undo changes made by collaborator who has disconnected due to interrupted connection Dec 7, 2018
@lostintangent
Copy link
Member

lostintangent commented Dec 8, 2018

@therealjawss Thanks for confirming that. Unfortunately, the issue you hit is an unintended result of the expected behavior in VS, because every participant has their own undo stack. In VS Code, we recently switched to a global undo stack, which is potentially something we could explore for VS as well.

This specific issue is clearly non-ideal, so we need to address it somehow. Let us give this some thought and get back to you. Thanks!

/cc @daytonellwanger

@lostintangent lostintangent changed the title [VS] Undo changes made by collaborator who has disconnected due to interrupted connection [VS] Changes can’t be undone that we’re made by participants who left the session Dec 8, 2018
@daytonellwanger
Copy link
Collaborator

Thanks for bringing this up @therealjawss . This is a situation we hadn't considered, and where having a global undo stack is crucial.

The difficulty is that there are also situations where having a local undo stack is crucial. So it seems that we need to have support for both local and global undo. I think the trickiest part is surfacing this functionality in a useful and intuitive way.

One easy solution: we provide both global and local undo, but under different keyboard shortcuts. e.g. ctrl+z for local undo, ctrl+z+g for global undo. Do you think this would be too burdensome and/or non-discoverable?

Perhaps a more discoverable and more functional version of this would be to add an "undo last change" item to the context menu that we show when you right-click the icon of one of the participants in a Live Share session, although this would require a significant amount of work on our end, and wouldn't have actually helped with the issue you ran into because the participant wouldn't show up in the list after they had left the session. In lieu of that, maybe we just add a single menu item to trigger global undo, which would undo the last change, regardless of who it was made by.

Another idea is that we could keep track of the last change that happened on each line, and when you click on a line we can provide a code action that would allow you to undo the last change that happened there (this also has the added advantage that you get to preview what the change will be). This is different than local/global undo (essentially each line has its own undo stack), but I feel like it may be more intuitive and useful. What are your thoughts about this idea (or the others)?

Please let us know if you have other thoughts on how we could support this.

@therealjawss
Copy link
Author

Is it possible to have a hybrid/dynamic stack? One that is local by default, but once the cursors intersect on lines they go global? Maybe it is a stack for a range of lines?

Live share is new to me and at this point I can really only think of a scenarios where people are collaborating on the same part of the code. I feel like if they are working on different parts of the code they should make use of source control and agree to sync often? :/

But how I imagine it working is if I'm working on one section and the collaborator for example Ctrl+a and deletes everything, since that intersected with my code, I should be able to Ctrl z to undo those changes too.

@daytonellwanger
Copy link
Collaborator

That's a great suggestion. We've considered this before and have been trying to come up with a good heuristic for determining what behavior (i.e. local vs. global undo) the user would expect. I think intersecting edits is a good one. We've also talked about checking to see if the other user's last edit was visible to you (if you didn't see the edit, you probably aren't attempting to undo it) and checking to see how far apart their edit was (in lines) from your last edit (maybe a threshold of something like 20 lines would imply that you're working separately). A line difference of 0 would be an intersecting edit.

A downside of this "dynamic undo" is that unless the user memorizes our set of heuristics, they're never going to be exactly sure what their undo will undo. And of course, since it's a heuristic, it will sometimes not be the desired behavior. Maybe it's worth the trade-off though.

As for scenarios where users work on different parts of the code, I think it's fairly common. Imagine you're working on a feature with someone, and you decide it requires two helper functions. You decide to tackle one and your collaborator tackles the other, and then you meet back up to finish the feature.

@therealjawss
Copy link
Author

therealjawss commented Dec 11, 2018 via email

@daytonellwanger
Copy link
Collaborator

Giving the host the ability to audit the changes that happened within a Live Share session is something we've been thinking about for a while, and this scenario is good motivation to increase the priority. I think 680 is the main issue we're using to track this work.

Thanks again for all the feedback, and please let us know if you have further ideas; this has been very helpful.

@daytonellwanger
Copy link
Collaborator

Copying over some feedback we received via a marketplace review.

From Conrado Fregonese Feltrin: "I tested with a friend in a C++ project, as the host of the live share, and it worked perfectly. The only problem was in a moment when I was doing Ctrl + z to undo something, and the smart one, in a joke, deleted all the code in the same file I was modifying. After that, my Ctrl + z didn't know what to do, but my friend could use Ctrl + z to recover everything."

@daytonellwanger
Copy link
Collaborator

Closing this issue and moving the conversation to 1439.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants