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

Including an add and a delete for the same file #241

Closed
nategreen opened this issue May 24, 2017 · 10 comments
Closed

Including an add and a delete for the same file #241

nategreen opened this issue May 24, 2017 · 10 comments

Comments

@nategreen
Copy link

nategreen commented May 24, 2017

My gulp process currently cleans the CSS directory before compiling Sass, which makes a delete and an add for the same file. I've been trying to add those two changes to the same commit. As I was about to post this issue I figured out that apparently, that's against the rules. Whatever, that's fine.

But, in the source control panel in VS code, nothing tells me that...the "include" option is enabled, there are no errors or anything when I try to include both changes. I click "include" and it just...does nothing, apparently. It would be nice if something had told me why it was just failing to do what I asked it to. 😄

Along those same lines, it would be nice to be able to select an add and a delete for the same file, and make them into an update. Doesn't look like TFVC can do that in VS2015, though, so maybe that would be harder than it's worth.

@jeffyoung
Copy link
Contributor

Thanks for trying out the extension @nategreen.

Can you clarify what you mean by "that's against the rules"? Do you mean that it doesn't work or it isn't supported? In this scenario, I presume the files have the same name?

Can you also clarify what you're asking it to do? Are there changes shown in the Excluded (or Included) Changes sections? Could you provide a screenshot? I'd like to see what gets rendered in the TFVC SCM Viewlet so I can have a better understanding of your scenario.

Finally, is there any additional information in the TFVC Output Window?

@nategreen
Copy link
Author

nategreen commented May 31, 2017

Hey! Sorry I haven't gotten back to this.

Can you clarify what you mean by "that's against the rules"?

Can't commit both an add and a delete for the same file. VS throws an error when I try to include both changes, so I assume it's just something that TFVC can't do. (Yes, named the same.) That makes sense, though.

Can you also clarify what you're asking it to do?

Sure. I can't reproduce it at the moment because for some reason it's not recognizing my TFVC project at all now...but here's what I was saying:

Best case: An add and a delete of the same file aren't two separate line items; the plugin just treats them as an edit/update instead.

Next-best case: When I include an add, any other changes for the same file have their "+" button (include) disabled, and when I hover it, I get a tooltip about why it's disabled (can't include multiple changes for the same file). Include in the right-click menu also disabled. Bonus points: when I have an add and a delete for the same file selected, give me an option (in right-click menu, for example) to merge those changes into an edit.

I'm not super familiar with TFVC in general, but I'm really not sure what would be the point of having both an add and a delete anyway; it seems obvious to me that if the file was deleted and then added again, the intention was a change. Seems like whatever changes I make to the codebase ought to be able to be committed at all times; not have to be committed twice to actually get it all in.

Does that make sense? I'll try to wrestle this thing into giving me TFVC as an option again, and then I can send you a screenshot, console output, etc.

@jeffyoung
Copy link
Contributor

Hi @nategreen. Thanks for the details (although I could certainly use a couple of screenshots so I can more fullly grasp your scenario).

I'm not familiar with Sass but after having a quick read it appears it's a superset of CSS that, when "compiled", produces CSS files. So it sounds like your system has CSS files already checked in (previously produced by Sass "compilation" (not sure if that's even the proper term)), the Sass process runs, deletes the previous CSS files and re-creates them. If that is accurate, then I would expect TFVC to behave in the following manner:

  • If the contents of the file did not change (there was no change to the Sass produced CSS file), there's no resulting pending change to that file. IOW, it would not appear in a tf status or in the TFVC SCM viewlet. As far as TFVC is concerned, the file didn't change.
  • If the contents of the file did change, and the file is not a new file, the change would be shown as an edit. It wouldn't matter that the file actually was deleted and re-created with new contents.
  • If the contents of the file did change, and the file got a new name, I would expect it to be shown as an add. The file that was deleted (with the old name) would be shown as a delete.

Based on what I described above, what you wrote ("it seems obvious to me that if the file was deleted and then added again, the intention was a change") is what I would expect to happen.

But if you get a screenshot or two, I'd be happy to see them.

@nategreen
Copy link
Author

image

Finally got one for you. Notice the mri-ui.css and mri-ui.min.css in both the included and excluded portions. If I try to include the add, that's when I get an error in VS and get no feedback whatsoever from VS Code.

image

@nategreen
Copy link
Author

And, when I hit include on one of those, here's what happens in TFVC output log...nothing much:

tf add -noprompt ******** c:\path\to\my\project\mri-ui.css
tf resolve -noprompt ******** c:\path\to\my\project -recursive -preview
tf status -noprompt -collection:http://tfs.mrisoftware.net/tfs/MRI ******** -format:detailed -recursive c:\path\to\my\project

@jeffyoung jeffyoung added the bug label Jun 2, 2017
@jeffyoung
Copy link
Contributor

Hey @nategreen thanks for the additional details and screenshots.

I took a closer look and this is certainly a bug in my TFVC SCM Provider. When we put this support together, I went ahead and added code to automatically process deletes. That is, if we see files that are 'candidate' deletes (files you just deleted yourself, for instance, or, say, a Saas compilation 😄) we promote them to actual deletes (what would happen if you ran tf delelete yourself). Once files are in that deleted state and those same files re-appear later, they're detected by tf status as adds. So that's why you see them both as a delete (since we incorrectly processed them as a delete) and then an add (TFVC sees them now as a candidate add).

I thought I might have a workaround for you by simply undoing the delete. However, that'll fail because the file it would replace has already been marked for add! 😞 And if you undo the add, you'll lose your changes! 😢 If there aren't that many changes in this state, for now, you could move the "add" files to a temp folder, undo the "deletes" and then drop the files back in. Those should then be detected as "edits". Certainly not ideal.

I'll go ahead and mark this as a bug and work on the changes needed to get it working. I think that'll entail adding a new menu item (similar to 'TFVC Rename' but 'TFVC Delete') and removing the code that does the automatic pending of candidate deletes. I'll need to make sure I don't regress other scenarios as well.

Thanks for hanging in there with me (so I could understand what's going on). I'll see if I can't get an update out early next week.

BTW, what error in did you get in Visual Studio when you tried it?

@nategreen
Copy link
Author

nategreen commented Jun 2, 2017

Oh, I see. That makes sense. Thanks for the explanation. 👍 Since I can just recompile the Sass it's no big deal for me to undo changes as a workaround...actually, my workaround was just to not clean the CSS directory for the time-being since there's such a low likelihood of stray files finding themselves there.

Thanks for hanging in there with me

Likewise. :)

BTW, what error in did you get in Visual Studio when you tried it?

When I attempt to "promote" the changes:

image

Apparently there are "multiple errors." haha. (I see this as Visual Studio saying to me, "oh, honey, what you just tried to do is wrong on so many levels.")

image

The output window says:

The item $/path/to/my/project/public/css/mri-ui.css already has pending changes.
The item $/path/to/my/project/public/css/mri-ui.min.css already has pending changes.

@jeffyoung
Copy link
Contributor

😃 Yeah, that's the message you'll get if you try to pend an add on a file that's already been marked for deletion. If our VS Code support showed this particular error in the log (you reported it doesn't), it would very likely be the same message as VS.

@jeffyoung
Copy link
Contributor

Hey @nategreen. I've attached a private build here (a VSIX file) if you'd like to try it out with your scenario. Here are the instructions on how to side-load this private build.

team-0.119.0.vsix.zip

I added a new right-click context menu in the File Explorer (so a user can delete via TFVC explicitly) and also tested a bunch of scenarios (e.g., deleting with TFVC delete then moving between include/exclude, deleting from file system then moving between include/exclude). If you're able, I'd appreciate you trying it out and letting me know if you get the results you expect.

@jeffyoung
Copy link
Contributor

I went ahead and pushed the changes necessary for the deletes to not be automatically promoted (and added a new menu item to the File Explorer for folks to use in order to do proper deletions of files). If files are deleted at the command prompt (outside of using the new menu item), they'll be excluded by default. When they're explicitly included (using the menu), we'll run tf delete then. That should fix it all up.

These changes are in v1.119.0 and it'd be great if you got a chance to try them out.

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

No branches or pull requests

2 participants