Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Add delete menu to file explorer #249

Merged
merged 1 commit into from
Jun 8, 2017
Merged

Add delete menu to file explorer #249

merged 1 commit into from
Jun 8, 2017

Conversation

jeffyoung
Copy link
Contributor

In this PR, we've removed the automatic promoting of file deletions from candidate to pending changes. This should address #241.

Tested scenarios:

  1. At command line, delete a versioned file via filesystem, re-create file with same name, shown as EDIT, defaults to included
    • move between excluded and included groups, works? yes
    • run "undo", works? yes
  2. At command line, delete a versioned file via filesystem, file shows in excluded changes, works? yes
    • move to included list (tf delete should get run on it), works? yes
    • move to excluded list, behaves as expected, works? yes
    • move back to included list (tf delete should NOT be run on it again), works? yes
  3. Use "Delete (TFVC)" menu item, item ends up in included list, works? yes
    • move to excluded list, works? yes
    • move back and forth between the two lists, works? yes
  4. Use "Delete (TFVC)" menu item, item ends up in included list, run undo, changes are undone? yes
  5. 'tf delete' does not show up on F1, works? yes

//Versioned changes should always be included
if (resource.IsVersioned) {
//Versioned changes should always be included (as long as they're not deletes)
if (resource.IsVersioned && !resource.HasStatus(Status.DELETE)) {
Copy link
Member

Choose a reason for hiding this comment

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

What happens if the deletes are just included? does the checkin fail?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If it's the only file in the Included section, yes. Basically, it's a 'detected change' and the check-in fails with this:

[Checkin] No changes were matched by any arguments.

Presumably, if there are other files in that Included list, those would get checked in. Without this change, if the file is versioned and they delete it via the command prompt or explorer, it would appear in the Included list but not be checked in. With this change, it'll be excluded and when they move it to included, we'll run tf delete on it.

}
} catch (err) {
//Provide a better error message if the file to be deleted isn't in the workspace (e.g., it's a new file)
if (err.tfvcErrorCode && err.tfvcErrorCode === TfvcErrorCodes.FileNotInWorkspace) {
Copy link
Member

Choose a reason for hiding this comment

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

Can we check ahead of time and just do a normal delete for them? I.e. so the user can ALWAYS call our delete.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We certainly could but then we'd want to also do it for rename. I just followed the rename pattern (if the file isn't in the workspace, we won't run a "tfvc" command on it). I don't have passion either way although we could do this and see if we get any feedback. Thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense. Let's keep it this way and get user feedback.

Copy link
Member

@jpricket jpricket left a comment

Choose a reason for hiding this comment

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

Couple comments but looks good!

@jeffyoung jeffyoung merged commit ba4cabe into master Jun 8, 2017
@jeffyoung jeffyoung deleted the jeyou/delete-menu branch June 8, 2017 17:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants