Auto checkout fails if you submit via P4V

[eddieparker]

Describe the bug:
If I use this addon and then submit via the P4V application, the next time I try to save the file in vscode, I get a "file is read-only" error. It's fixable if you 'refresh' within the SCM dialog in vscode, then saving will check out the file automatically for you and save it.

Expected behavior:
In the case that it finds a read only file it would be nice if it did a quick refresh automatically and then tried to re-checkout.

To Reproduce:
Install the addon
Check out a file and edit it
Submit it via P4V (not VScode)
Try to edit the file in vscode and save it
You'll get the error I mention.

Versions & Details:

• Perforce extension: 4.12.0
• VSCode version: 1.47.3
• Operating System: Windows 10
• Perforce server details (if known): Unknown
• Are you using a multi-root workspace?: No

[mjcrouch]

Hi, thanks for reporting.

I assume you have the "edit on save" setting enabled so that it should open the file on save?

While we can't really make guarantees about what happens when you use external tools at the same time, I think I've noticed this happen when submitting within VS code as well, possibly if you don't switch to a different editor tab and back after submitting.

IIRC we don't call the edit command if we already think the file is open for edit, to prevent spamming the perforce server, particularly if you have auto save enabled in vs code.

Possibly we can check the file system to see if the file is read only or not at save-time. However on a very brief look, I can't find anything in the vs code API to check whether a file is writable. Potentially we could use an in-built node module, but I think this is discouraged in vs code for various reasons.

So, will need to do a bit of digging about what we can do to improve this behaviour without hitting the perforce server too much.

[eddieparker]

Yes, I have the edit-on-save command set; and aside from this minor issue, it's great!

Thank you for the detailed response! This issue is obviously workaroundable but is a mild annoyance, so I definitely appreciate you looking into it.

Dumb question; no chance the VScode API gives your addon a callback when it tries to write the readonly file and then you could try editing it? Or it's too late by then?

[mjcrouch]

Well, to be a little more specific, we already register a callback which gets called when a save is 'about to happen', that's where we hook in to the to run the p4 edit command currently. We basically have the power to tell vs code not to save the file until we're done with our perforce command (up to some arbitrary time limit). At this point obviously the file is readonly but vs code never explicitly tells us about the readonly problem. I imagine it doesn't know itself until it tries to write the file, at which point it catches an error and displays a warning to the user. There aren't any later hooks that we can use to react to that.

Currently when we receive this callback we just ignore any files that we already think are open, so that we don't hit perforce or hold up the save command unecessarily. Note that if you switch to a different document and back, it should recognise that the file's current status doesn't match up with the status in the SCM provider, mark that file as 'conflicting' and actually just do the edit anyway.

One possible option - I don't know if this is actually possible or sensible - is to watch for changes in the file system (there's an API for that), and mark any files that have been changed externally as 'conflicting' and then just try to edit them anyway. The reason I say this might not be possible / sensible is I suspect that changes made within vs code will still trigger these events and we'll end up doing this for everything.

I think using fs.access from node to check the file system permissions may be a good option generally but my concern is that since we're holding up the save in the first place it's best to do as little work as possible in these callbacks, and as mentioned using the built in fs module is not recommended by vs code, mainly because sometimes the URIs that we receive don't actually represent file system paths. Though in reality that's probably not going to be the case for perforce stuff.

[mjcrouch]

coincidentally microsoft/vscode#91697 and the associated PR might give us enough information to implement a simple check at save-time. Though it might not catch every case we should be able to check at the very least the current activeTextEditor to see if it's the file being saved and if so whether it's readonly or not.

[mjcrouch]

looks like the proposed API in the issue above was actually the much less useful check whether a filesystem scheme is generally writable - so not convinced this is going to be do-able any time soon