Skip to content
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

Unparsing FileInfo and DirectoryInfo should add quotes to the path #626

Closed
kapsiR opened this issue May 13, 2020 · 5 comments · Fixed by #627
Closed

Unparsing FileInfo and DirectoryInfo should add quotes to the path #626

kapsiR opened this issue May 13, 2020 · 5 comments · Fixed by #627

Comments

@kapsiR
Copy link
Contributor

kapsiR commented May 13, 2020

FormatWithQuotesIfString currently honors DateTime, DateTimeOffset and string, but I'm using FileInfo and DirectoryInfo as options as well.

These types should also be surrounded by quotation marks when unparsed.

.NET Fiddle example

Affected code:

private static object FormatWithQuotesIfString(object value)
{
if (value is DateTime || value is DateTimeOffset) return $"\"{value}\"";
Func<string, string> doubQt = v
=> v.Contains("\"") ? v.Replace("\"", "\\\"") : v;
return (value as string)
.ToMaybe()
.MapValueOrDefault(v => v.Contains(' ') || v.Contains("\"")
? "\"".JoinTo(doubQt(v), "\"") : v, value);
}

@kapsiR kapsiR changed the title UnParsing FileInfo and DirectoryInfo should add quotes to the path Unparsing FileInfo and DirectoryInfo should add quotes to the path May 13, 2020
@moh-hassan
Copy link
Collaborator

To support any object that may be string with spaces, i think FormatWithQuotesIfString need to be re-written with minor change as:

 private static object FormatWithQuotesIfString(object value)
        {
           // if (value is DateTime || value is DateTimeOffset || value is FileInfo || value is DirectoryInfo) return $"\"{value}\"";
            var s = value.ToString();
            if (!string.IsNullOrEmpty(s) && !s.Contains("\"") && s.Contains(" "))
                return $"\"{s}\"";


            Func<string, string> doubQt = v
                => v.Contains("\"") ? v.Replace("\"", "\\\"") : v;

            return s.ToMaybe()
                .MapValueOrDefault(v => v.Contains(' ') || v.Contains("\"")
                    ? "\"".JoinTo(doubQt(v), "\"") : v, value);
        }

your test cases are passed using this func.

If you agree, you can modify PR and replace this code.

@kapsiR
Copy link
Contributor Author

kapsiR commented Jun 15, 2020

@moh-hassan Thanks! I changed the PR accordingly.
I hope there will be no case in the future where the simple ToString() will cause issues here 😉

@moh-hassan
Copy link
Collaborator

@kapsiR
The method ToString() is implemented in base object of every Value Type or Reference type (all inherit from object) and it's override by most of built-in types like: DateTime, DateTimeOffset, FileInfo, DirectoryInfo, ....
.

@kapsiR
Copy link
Contributor Author

kapsiR commented Jun 16, 2020

@moh-hassan Yeah, I'm aware of that and that's the reason why I mentioned it, because we are just looking for a whitespace.
If there would be anyone who overrides a ToString() with a custom implemenation that has a whitespace, it would be treated the same. (maybe unintended)

[EDIT]
Of course, this shouldn't be a real issue here 😀

@moh-hassan
Copy link
Collaborator

Sure 😄

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

Successfully merging a pull request may close this issue.

2 participants