vscode-hediet-power-tools icon indicating copy to clipboard operation
vscode-hediet-power-tools copied to clipboard

Extension clobbers global gitconfig setting

Open AjayKMehta opened this issue 3 years ago • 9 comments

I was wondering why the core.excludesFile setting in my global .gitconfig file was getting clobbered and I tracked it down to this.

I had the value in the file set to C:\\users\\ajay\\.gitconfig.

git config --get core.excludesFile returns c:\users\ajay\.gitconfig.

The issue I believe is that existsSync does not handle Window-style paths correctly (not verified). I also saw that it does not handle ~ in file path.

Can you please fix this so it handles these cases correctly? For now, I switched to Unix style path to be safe.

Thanks.

AjayKMehta avatar Feb 10 '22 16:02 AjayKMehta

Thanks for reporting this!

What is the expected value of core.excludesFile and what is the actual value?

What is your suggestion to fix this?

hediet avatar Feb 10 '22 18:02 hediet

So, the value in the .gitconfig file has \\ as path separator but as stated in my previous comment, git config --get core.excludesFile returns c:\users\ajay.gitconfig with \ as separator which existsSync does not like.

Screenshot below illustrates this as well as issue with ~:

existsSync

My suggestion is to replace single \ with \\ before calling this method, replace ~ using advice from accepted answer here or use a different library.

I'm not really familiar with JavaScript or TypeScript or else, I would have happily submitted a PR to fix this 😀

Thank you for making such a great extension that I use daily!

AjayKMehta avatar Feb 10 '22 18:02 AjayKMehta

with \ as separator which existsSync does not like.

This cannot be the issue.

When you provide a string literal in JS, you just have to double enquote \. \u starts a unicode sequence, but digits have to follow.

I.e. "C:\\users" literally represents the string C:\users.

hediet avatar Feb 11 '22 09:02 hediet

Yes, you are right. I should have been more specific saying that single \ is a problem when followed by certain characters such that it forms an escape sequence.

For example, \n is also a problem:

fs.existsSync("C:\program files\notepad++") returns false. fs.existsSync("C:\\program files\\notepad++") returns true.

Solution is to replace single \ with \\ or / in return value of git config --get core.ExcludesFile.

AjayKMehta avatar Feb 11 '22 16:02 AjayKMehta

I have the same issue. Even when I'm editing .git-global-excludes-file it's still gets replaced every time. I'm on macOS.

@hediet, I think the issue comes down to

  1. not stripping stdout, so fs.existsSync("~/.gitignore\n") is called
  2. as @AjayKMehta mentioned, fs.existsSync doesn't expand ~/ in path

So solution would be:

  1. call stdout.trim()
  2. expand ~/ if found in path
$ node
Welcome to Node.js v18.2.0.
Type ".help" for more information.
> const { promisify } = require('util');
undefined
> const { exec } = require('child_process');
undefined
> const { stdout } = await promisify(exec)(
... 			"git config --get core.excludesfile"
... 		)
undefined
> stdout
'~/.gitignore\n'
>
@AjayKMehta, you seem to misunderstand how escaping works.

I'm familiar with python, so will write an example using it, however same applies to JS, AFAIK.

When you hardcode strings, you have to escape certain characters because they would be replaced by interpreter otherwise:

var = "C:\\program files\\notepad++"
print(var)  # C:\program files\notepad++

But when you're dealing with input, you don't have to worry about, say, \n sequences, because it would literally represent a \ followed by n, same as you would get by hardcoding "\\n":

var = input()
print(var)  # C:\program files\notepad++

Bobronium avatar Aug 15 '22 12:08 Bobronium

@Bobronium thanks for your analysis, can you do a PR? :)

hediet avatar Aug 15 '22 13:08 hediet

Thanks for the primer on escaping, @Bobronium.

I understand how escaping works but not how it is done in JS/TS 😃

A PR to fix this would be great.

AjayKMehta avatar Aug 15 '22 14:08 AjayKMehta

A PR to fix this would be great.

There is one!

Bobronium avatar Aug 15 '22 15:08 Bobronium

Awesome 👍

On Mon, Aug 15, 2022, 9:15 AM Arseny Boykov @.***> wrote:

A PR to fix this would be great.

There is one!

— Reply to this email directly, view it on GitHub https://github.com/hediet/vscode-hediet-power-tools/issues/10#issuecomment-1215139957, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACVJQJ7LXXFNKQJNTCA7KV3VZJNHPANCNFSM5OBGMNQQ . You are receiving this because you were mentioned.Message ID: @.***>

AjayKMehta avatar Aug 15 '22 15:08 AjayKMehta