One page design for FileSavePicker.SuggestedSaveFilePath

[DinahK-2SO]

One page design for FileSavePicker.SuggestedSaveFilePath

Code implementation: #5547

[benstevens48]

Why have a suggested file instead of folder? What if we want the initial file name to be blank, in case there is no good default name or we want to force the user to choose a name?

[codendone]

Looks good to me. Per @benstevens48's question, if SuggestedSaveFile only points at a folder instead of a file in a folder, will that have the effect of setting the start folder with no default filename? Or is it required to include a suggested filename?

[DinahK-2SO]

Looks good to me. Per @benstevens48's question, if SuggestedSaveFile only points at a folder instead of a file in a folder, will that have the effect of setting the start folder with no default filename? Or is it required to include a suggested filename?

Thanks @codendone for reviewing this design!

First, The SuggestedSaveFile of UWP FileSavePicker doesn't accept a StorageFolder object - the compiler would show this error

Cannot implicitly convert type 'Windows.Storage.StorageFolder' to 'Windows.Storage.StorageFile'

In the scope of our new APIs, I would strongly recommend the logic of TrySuggestedSaveFilePath to be simple as below:

TrySuggestedSaveFilePath takes whatever after the last slash to fill in the file name box,
as long as the part before last slash is a valid and existing folder.
And return false without changing any value if the input doesn't even contain a slash, or the folder doesn't exist.
Test cases here for clarification, will update the content in this design next.

The input would be largely depends on API consumer, but above logic is concrete - The method itself doesn't decide if the input path is a file path or folder path.

This is to reduce the complexity and avoid more undefined behaviors (e.g. should .git in C:\temp\MyProject\.git be treated as a filename or a folder name? what if the input string is C:\temp\1.txt but there's folder 1.txt under C:\temp? etc. )

For the questioned scenario, we can add another method in future, to set the suggested folder without file name, e.g. introducing a SuggestedFolder attribute and TrySetSuggestedFolder, and look for the folder of input path string, no matter the format.

[codendone]

TrySuggestedSaveFilePath takes whatever after the last slash to fill in the file name box, as long as the part before last slash is a valid and existing folder. And return false without changing any value if the input doesn't even contain a slash, or the folder doesn't exist.

I agree with that design.

This is to reduce the complexity and avoid more undefined behaviors (e.g. should .git in C:\temp\MyProject\.git be treated as a filename or a folder name? what if the input string is C:\temp\1.txt but there's folder 1.txt under C:\temp? etc. )

This relates to the question "if SuggestedSaveFile only points at a folder instead of a file in a folder, will that have the effect of setting the start folder with no default filename? Or is it required to include a suggested filename?".

From the implementation, I believe the current design is if C:\temp\MyProject\.git\ is specified with the trailing \, then a folder is specified and will be used (assuming it exists) and no filename will be set (allowing SuggestedSaveFile to be used, if that is set). I think that design is good.

But, as you said, what happens if C:\temp\MyProject\.git is set? I think the current implementation will use .git as the default filename even if that exists as a folder. That seems wrong. It may be necessary to check if the full specified path points to a folder and treat it as a folder with no filename.

Splitting out a separate SuggestedFolder attribute and TrySetSuggestedFolder seems unnecessary (and potentially confusing) if the handling of SuggestedSaveFilePath simply does that extra "the path is just a folder" check.

[DinahK-2SO]

But, as you said, what happens if C:\temp\MyProject\.git is set? I think the current implementation will use .git as the default filename even if that exists as a folder. That seems wrong. It may be necessary to check if the full specified path points to a folder and treat it as a folder with no filename.

Thanks @codendone for raising this point! I also discussed this topic with @yeelam-gordon yesterday, and we concluded the current design could be more beneficial.

It's great to know that we have reached consensus on the first case:

• C:\temp\MyProject\.git\ sets folder C:\temp\MyProject\.git, with empty file name; (Thanks @benstevens48 for raising the use scenario, with this tailing \, developer can set folder while leaving the initial file name blank)

The question is the second case:

• C:\temp\MyProject\.git sets folder C:\temp\MyProject, with .git filled into file name, even when there's a folder C:\temp\MyProject\.git.

While it might seem counter-intuitive at first, this behavior is based on a very strict logic. It makes the API's behavior 100% predictable and gives developers clearer control over the code.

• Via TrySetSuggestedSaveFilePath, the developer attempts to pass 2 suggestions to the end user: a location and a file name.

  With the current design, if the folder C:\temp\MyProject\.git exists, end user can see:

  1. the save dialog opens C:\temp\MyProject (passing the 1st suggestion).
  2. it is also suggested to save the new file as .git (the 2nd suggestion), although they cannot save as there's already a folder named .git

  In this case, the end user can decide:

  1. rename the folder .git to another name and save file
  2. change the new file's name
  3. or choose any other location with any other file name

  All based on 2 suggestions from the developer.

• In contrast, if we check the folder existence of the input string and jump into the deeper folder, the API's behavior would become more complex and unpredictable. In above example, end user would see:

  1. the save file dialog opens C:\temp\MyProject\.git, which was not the intentioned folder of developer,
  2. no suggested file name - another information failed to pass to the end user.

  This would be like a second-guessing the developer's intent based on the unpredictable state of the end-user's file system, bringing an unreliable API's behavior

@jonwis, thank you for reviewing the PR. As we have agreed to use the string attribute SuggestedSaveFilePath, I am marking your comments as resolved.

[DinahK-2SO]

/azp run

[azure-pipelines]

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.