GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Add a path for checking ZoneInformation without throwing an exception by powercode · Pull Request #8025 · PowerShell/PowerShell

Conversation

@powercode

@powercode Staffan Gustafsson (powercode) commented Oct 14, 2018 •
edited by iSazonov
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Collaborator

PR Summary

In an effort to reduce the number of exceptions we throw as standard flowcontrol, this PR adds

bool TryCreateFileStream(
    string path,
    string streamName, 
    FileMode mode, 
    FileAccess access, 
    FileShare share,  
   out FileStream stream) 

as a non-throwing alternative to CreateFileStream.

This makes the code slightly faster and debugging slightly less annoying.

PR Checklist

  • PR has a meaningful title
    • Use the present tense and imperative mood when describing your changes
  • Summarized changes
  • Change is not breaking
  • Make sure all .h, .cpp, .cs, .ps1 and .psm1 files have the correct copyright header
  • This PR is ready to merge and is not Work in Progress.
    • If the PR is work in progress, please add the prefix WIP: to the beginning of the title and remove the prefix when the PR is ready.
  • User-facing changes
    • Not Applicable
    • OR
    • User-facing Documentation needed
      • Issue filed - Issue link:
  • Testing - New and feature
    • Not Applicable or can only be tested interactively
    • OR
    • Make sure you've added a new test if existing tests do not effectively test the code changed
      • Add [feature] if the change is significant or affects feature tests

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@powercode
Add exception free standard path for checking zone identifier
0f31976
@powercode
Staffan Gustafsson (powercode) requested review from Bruce Payette (BrucePay) and Andrew (anmenaga) as code owners October 14, 2018 18:20
iSazonov
Ilya (iSazonov) reviewed Oct 15, 2018
View reviewed changes
Comment thread src/System.Management.Automation/namespaces/FileSystemProvider.cs Resolved

Uh oh!

There was an error while loading. Please reload this page.

@powercode
Addressing Ilya's comments.
d5dc2bb
iSazonov
Ilya (iSazonov) reviewed Oct 16, 2018
View reviewed changes
Comment thread src/System.Management.Automation/namespaces/FileSystemProvider.cs Outdated
string errorMessage = StringUtil.Format(
FileSystemProviderStrings.AlternateDataStreamNotFound, streamName, path);
throw new FileNotFoundException(errorMessage, resultPath);
throw new FileNotFoundException(errorMessage, $"{path}:{streamName.Trim()}");

@iSazonov Ilya (iSazonov) Oct 16, 2018

Copy link
Copy Markdown
Collaborator

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.

Do we really need Trim() for streamName in the internal methods? We do not this for path.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@powercode Staffan Gustafsson (powercode) Oct 16, 2018

Copy link
Copy Markdown
Collaborator Author

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.

Not sure - I just opted to not change any functionality. Small cost on the error path.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@iSazonov Ilya (iSazonov) Oct 16, 2018

Copy link
Copy Markdown
Collaborator

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.

I vote to remove - I believe consumer should take care.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@powercode
Removing Trim call on streamName.
fa724c4
iSazonov
Ilya (iSazonov) reviewed Oct 17, 2018
View reviewed changes
Comment thread src/System.Management.Automation/namespaces/FileSystemProvider.cs Resolved

Uh oh!

There was an error while loading. Please reload this page.

Ilya (iSazonov) added 2 commits October 17, 2018 23:44
@iSazonov @powercode
Removing the second trim call.
13163fb
@iSazonov
Use nameof()
afbb6c7
iSazonov
Ilya (iSazonov) approved these changes Oct 23, 2018
View reviewed changes
@iSazonov
Ilya (iSazonov) requested review from Steve Lee (SteveL-MSFT) and Aditya Patwardhan (adityapatwardhan) October 23, 2018 11:53
@iSazonov Ilya (iSazonov) self-assigned this Oct 23, 2018
anmenaga
Andrew (anmenaga) approved these changes Oct 23, 2018
View reviewed changes
@iSazonov
Ilya (iSazonov) merged commit 3e4fa87 into PowerShell:master Oct 24, 2018
TravisEz13
Travis Plunk (TravisEz13) reviewed Nov 14, 2018
View reviewed changes

@TravisEz13 Travis Plunk (TravisEz13) left a comment

Copy link
Copy Markdown
Member

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.

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj-contribs/PowerShell that referenced this pull request Aug 6, 2026
@powercode @iSazonov
Add a path for checking ZoneInformation without throwing an exception (…
f459c95
…PowerShell#8025)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TravisEz13 Travis Plunk (TravisEz13) TravisEz13 left review comments
@iSazonov Ilya (iSazonov) iSazonov approved these changes
@BrucePay Bruce Payette (BrucePay) Awaiting requested review from BrucePay
@adityapatwardhan Aditya Patwardhan (adityapatwardhan) Awaiting requested review from adityapatwardhan
@SteveL-MSFT Steve Lee (SteveL-MSFT) Awaiting requested review from SteveL-MSFT
+1 more reviewer
@anmenaga Andrew (anmenaga) anmenaga approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@iSazonov Ilya (iSazonov)

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@powercode @TravisEz13 @anmenaga @iSazonov
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.