I am not very keen on changing the fsPath-semantics for the URI type because that is used a lot and worst, values of fsPath might be cached and must remain stable... @LeonardoBraga Can't you make the fix at a less generic level but the place in which the .gitignore file is saved?

@jrieken I completely understand your concern, and that was not my initial attempt as well. I've been working on the codebase for just a few days, so I might obviously be missing something, but let me explain the reasoning behind the fix and then we could try to find a better path. Sorry for the long reply.

Assuming you have the following scenario:

• Empty folder located at \\some-server\some-share\test
• User executed git init inside test
• User created only one file, a.txt
• The folder does not have a .gitignore file

In the current flow, when the user opens the folder \\some-server\some-share\test in VSCode, goes to the SCM panel, right clicks a.txt and selects Add to .gitignore, VSCode will run the logic in https://github.com/Microsoft/vscode/blob/master/extensions/git/src/repository.ts#L876, that will check whether .gitignore already exists. It doesn't, so the document const will contain a reference to a URI with the scheme untitled. This allows the editor to open with a file that is still in-memory (it does not have the scheme set to file yet). As a user of VSCode, I really like this design decision, as it lets me "confirm" my action by saving the file after I evaluate the changes, or just close it without saving, going back to the state I was in.

The problem here is that the code does not support constructing the fsPath of a UNC resource from a URI that has a scheme different than file.

• First, I tried to force the scheme to be file instead of untitled, which fails as the file is not yet persisted. I gave up that path as it felt like I would need to change the semantics behind the scheme file to make it work before the file actually exists in the disk.
• Then I thought of creating the .gitignore file immediately, which would allow me to have a URI with the scheme file and the rest would just work, but this approach breaks the existing design and removes the ability for the user to go back to the previous state by just closing the file without saving it.
• Then I got to the code in this PR. Even though it is an internal change, based on what I saw in the code, the scheme only transitions from untitled to file, which would not be a problem. Also, from what I could gather, all the use cases where scheme and authority are involved besides UNC, are related to URLs (http or https), so I didn't see any risks. An added "bonus" is that if any similar use cases of in-memory files over UNC paths are required now or in the future, this improvement would enable them transparently.

Let me know if any of the other approaches sound better to you, or if you know about any other possible paths I could explore, I will definitely give them a shot.

Let me know if any of the other approaches sound better to you, or if you know about any other possible paths I could explore,

Yeah, don't get me wrong. The change does look good but I am extremely conservative when it comes to uri-changes. It's huge source of bikeshedding and there will be many people arguing withs loads of passion why that change isn't good.

So, I wonder if we can tackle this at the place in which an untitled file is being saved. Somewhere (@bpasero should know where) the untitled-uri must be turned into a file-uri and instead of calling fsPath it should construct an fsPath-equivalent from the components of the uri itself.

could be this file of code: https://github.com/Microsoft/vscode/blob/b7e6e04666ca8dde560b69081c43365dce77d12a/src/vs/workbench/services/textfile/common/textFileService.ts#L446

Reminds me of this series of debt-issues we went through: https://github.com/Microsoft/vscode/issues?utf8=✓&q=is%3Aissue+fsPath+%22Don%27t+call+URI%23fsPath%22+

Just double-checking before implementing your suggestion, the use case I described above is also replicated in https://github.com/Microsoft/vscode/blob/master/extensions/typescript-language-features/src/utils/tsconfig.ts#L63-L64, and as the screenshot below shows, this patch also fixes that:

@jrieken Do you still prefer to address this use case on a case-by-case approach instead of a generic solution? If so, I no worries, I'll submit a new patch.

/cc @bpasero

Yeah, I believe a fix in the service for untitled files will also cover the TypeScript part.

Understood! Let me work on that and I'll update the PR.

@bpasero @jrieken I isolated the changes around TextFileService, as suggested. Let me know if you have any additional feedback.

Looks good to me but I'll leave it up to @bpasero

@bpasero completely agreed with the recommendation and thanks for pointing out another place that needed the fix. I've updated the PR with the changes you suggested.

@LeonardoBraga thanks 👍