-
-
Notifications
You must be signed in to change notification settings - Fork 38
fix: handle file paths with spaces in git commands #156
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 4 commits
8b14cce
dec5f76
459c800
410745c
30449b7
3c52cbe
6ecc630
f391cd3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,8 +3,8 @@ | |
| "variables": [ | ||
| { | ||
| "name": "root-dir", | ||
| "command": "cmd", | ||
| "args": ["/c", "dir", "/b"] | ||
| "command": "ls", | ||
alirezanet marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| "args": [] | ||
| } | ||
| ], | ||
| "tasks": [ | ||
|
|
@@ -21,9 +21,9 @@ | |
| { | ||
| "name": "echo staged files", | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why this test task is removed?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Restored in commit 6ecc630. Updated to use cross-platform |
||
| "pathMode": "absolute", | ||
| "command": "cmd", | ||
| "command": "echo", | ||
alirezanet marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| "group": "pre-commit", | ||
| "args": [ "/c", "echo", "${staged}"] | ||
| "args": ["${staged}"] | ||
| } | ||
| ] | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,6 +26,23 @@ public async Task<BufferedCommandResult> ExecBufferedAsync(string fileName, stri | |
| } | ||
| } | ||
|
|
||
| public async Task<BufferedCommandResult> ExecBufferedAsync(string fileName, IEnumerable<string> args) | ||
| { | ||
| try | ||
| { | ||
| var result = await CliWrap.Cli | ||
| .Wrap(fileName) | ||
| .WithArguments(args) | ||
| .ExecuteBufferedAsync(); | ||
| return result; | ||
| } | ||
| catch (Exception) | ||
| { | ||
| $"failed to execute command '{fileName}'".LogErr(); | ||
| throw; | ||
| } | ||
| } | ||
|
Comment on lines
+29
to
+44
|
||
|
|
||
| public async ValueTask SetExecutablePermission(params string[] files) | ||
| { | ||
| if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) | ||
|
|
@@ -53,6 +70,18 @@ public async Task<CommandResult> ExecDirectAsync(string fileName, string args) | |
| return result; | ||
| } | ||
|
|
||
| public async Task<CommandResult> ExecDirectAsync(string fileName, IEnumerable<string> args) | ||
| { | ||
| var result = await CliWrap.Cli | ||
| .Wrap(fileName) | ||
| .WithArguments(args) | ||
| .WithValidation(CommandResultValidation.None) | ||
| .WithStandardOutputPipe(PipeTarget.ToDelegate(q => q.Log())) | ||
| .WithStandardErrorPipe(PipeTarget.ToDelegate(q => q.LogErr())) | ||
| .ExecuteAsync(); | ||
| return result; | ||
| } | ||
|
|
||
| public async Task<CommandResult> RunCommandAsync( | ||
| string fileName, | ||
| IEnumerable<string> args, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,4 @@ | ||
| using System.IO.Abstractions; | ||
| using System.Runtime.InteropServices; | ||
| using System.Text.RegularExpressions; | ||
| using CliFx.Exceptions; | ||
| using Husky.Services.Contracts; | ||
|
|
@@ -107,7 +106,7 @@ | |
| foreach (var tf in tmpFiles) | ||
| { | ||
| // add formatted temp file to git database | ||
| var result = await _git.ExecBufferedAsync($"hash-object -w {tf.tmp_path}"); | ||
| var result = await _git.ExecBufferedAsync(new[] { "hash-object", "-w", tf.tmp_path }); | ||
|
Check warning on line 109 in src/Husky/TaskRunner/StagedTask.cs
|
||
| var newHash = result.StandardOutput.Trim(); | ||
|
|
||
| // check if the partial hash exists | ||
|
|
@@ -122,7 +121,7 @@ | |
| { | ||
| $"Updating index entry for {tf.src_path}".LogVerbose(); | ||
| await _git.ExecAsync( | ||
| $"update-index --cacheinfo {tf.dst_mode},{newHash},{tf.src_path}" | ||
| new[] { "update-index", "--cacheinfo", $"{tf.dst_mode},{newHash},{tf.src_path}" } | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. use new c# syntax with:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated in commit 6ecc630. Now using C# 12 collection expression syntax:
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see it resolved in line 124 |
||
| ); | ||
| } | ||
| else | ||
|
|
@@ -143,14 +142,18 @@ | |
| .OfType<FileArgumentInfo>() | ||
| .Where(q => q.ArgumentTypes == ArgumentTypes.StagedFile) | ||
| .Except(partialStagedFiles) | ||
| .Select(q => !RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? $"\"{q.RelativePath}\"" : $"\"{q.RelativePath.Replace("/", @"\")}\"") | ||
| .Select(q => q.RelativePath) | ||
| .ToList(); | ||
|
|
||
| if (stagedFiles.Any()) | ||
| { | ||
| "Re-staging staged files...".LogVerbose(); | ||
| string.Join(Environment.NewLine, stagedFiles).LogVerbose(); | ||
| await _git.ExecAsync($"add {string.Join(" ", stagedFiles)}"); | ||
|
|
||
| // Build git add command with file paths as separate arguments | ||
| var gitAddArgs = new List<string> { "add" }; | ||
| gitAddArgs.AddRange(stagedFiles); | ||
| await _git.ExecAsync(gitAddArgs); | ||
| } | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why this test variable is removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Restored in commit 6ecc630. Updated to use cross-platform commands (
bash -c lsinstead ofcmd /c dir /b) to work on Linux/CI environments.