Skip to content

Fix auto-downloading of files in OneDrive when doing dir #9895

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.

Sign up for GitHub

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

Merged
iSazonov merged 1 commit into from
Jul 8, 2019
Merged

Fix auto-downloading of files in OneDrive when doing dir #9895

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/System.Management.Automation/namespaces/FileSystemProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2001,7 +2001,8 @@ string ToModeString(FileSystemInfo fileSystemInfo)
public static string NameString(PSObject instance)
{
return instance?.BaseObject is FileSystemInfo fileInfo
? InternalSymbolicLinkLinkCodeMethods.IsReparsePoint(fileInfo)
? (InternalSymbolicLinkLinkCodeMethods.IsReparsePoint(fileInfo) &&
InternalSymbolicLinkLinkCodeMethods.IsNameSurrogateReparsePoint(fileInfo.FullName))
Copy link
Collaborator

@iSazonov iSazonov Jun 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that we use IsReparsePoint() in 3 places and with the fix in 2 places we use IsReparsePoint() with IsNameSurrogateReparsePoint(). So question is should we do the same in
private static string Mode(PSObject instance, bool excludeHardLink) too? If yes make sense join both the methods in one?

Copy link
Member Author

@SteveL-MSFT SteveL-MSFT Jun 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw that as well. In the 3rd case, it's used to construct the Mode property. I'm fine leaving that as-is so that an online file still shows up as l to indicate it is a link of some type. Alternative would be to introduce a o to indicate online, but that may be confusing to users.

Copy link
Collaborator

@iSazonov iSazonov Jun 14, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My thoughts was that if user calls Mode property to display we will download the file in the same way as with NameString(). Did you the check?

Copy link
Contributor

@sba923 sba923 Jun 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course, this is a matter of taste, but I tend to favor using "o" over "l" because "a link of some type" is also somewhat confusing--I prefer being able to easily tell an online file from a symlink.

Copy link
Member Author

@SteveL-MSFT SteveL-MSFT Jul 1, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's have @PowerShell/powershell-committee discuss this

Copy link
Member Author

@SteveL-MSFT SteveL-MSFT Jul 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@PowerShell/powershell-committee reviewed this. Currently, the length is in parenthesis already if the file is not downloaded locally so there doesn't seem to be a need to change the left column of mode introducing a new value. Instead our recommendation is to continue to use l to show it is a link.

Copy link
Collaborator

@iSazonov iSazonov Jul 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SteveL-MSFT Could you please address my comment about Mode property?

Copy link
Member Author

@SteveL-MSFT SteveL-MSFT Jul 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iSazonov Mode is displayed by default and doesn't incur a download of the file.

? $"{fileInfo.Name} -> {InternalSymbolicLinkLinkCodeMethods.GetTarget(instance)}"
: fileInfo.Name
: string.Empty;
Expand Down