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 — Update 'SaveError' in 'Parser.cs' to not depend on LINQ expression tree to get the error id · Issue #6444 · PowerShell/PowerShell

Update 'SaveError' in 'Parser.cs' to not depend on LINQ expression tree to get the error id #6444

New issue
Copy link
New issue
Copy link
Closed
#6498
Closed
Update 'SaveError' in 'Parser.cs' to not depend on LINQ expression tree to get the error id#6444
#6498
Copy link
Assignees
rjmholt
Labels
Issue-Code Cleanupthe issue is for cleaning up the code with no impact on functionalitythe issue is for cleaning up the code with no impact on functionalityResolution-FixedThe issue is fixed.The issue is fixed.

Description

@daxian-dbw
Dongbo Wang (daxian-dbw)
opened on Mar 20, 2018
Issue body actions

Quote Jason's comment at https://github.com/PowerShell/PowerShell/pull/6434/files/5b5be3122971552d2ce208858ac84868954b841c#r175933192

I'd suggest removing my ugly hack of using LINQ expression trees in SaveError and related helper methods, and replace with nameof(ParserStrings.MissingEndCurlyBrace).

When that code was written, nameof was not in C#, so the ugly and inefficient code was used because:

  • It ensured no typos in the FullyQualifiedErrorId.
  • It was easy to search for all references to an error message via the property.
  • Performance was not deemed important because they were errors.

Performance might matter more than I originally assumed given editor scenarios - and given the nameof language feature, it's reasonable (but annoying and a little verbose) to change error reporting to look more like:

ReportIncompleteInput(lCurly.Extent, rCurly.Extent,
    ParserStrings.MissingEndCurlyBrace, nameof(ParserStrings.MissingEndCurlyBrace));

In other words, pass the error text and the error id.

Alternatively you could just pass the error id and use reflection of some sort to load the error string, but that would not be good for performance.

Activity

  1. Dongbo Wang (daxian-dbw)
    changed the title [-]Update 'SaveError' in 'Parser.cs' to not depend on 'Expression<Func<string>>' to get the error id[/-] [+]Update 'SaveError' in 'Parser.cs' to not depend on LINQ expression tree to get the error id[/+] on Mar 20, 2018
  2. Ilya (iSazonov)
    added
    Issue-Code Cleanupthe issue is for cleaning up the code with no impact on functionalitythe issue is for cleaning up the code with no impact on functionality
    on Mar 21, 2018
  3. Rob Holt (rjmholt)
    assigned
    Rob Holt (rjmholt)
    and unassigned
    Rob Holt (rjmholt)
    on Mar 23, 2018
  4. Dongbo Wang (daxian-dbw)
    closed this as completedin #6498on Mar 28, 2018
  5. Ilya (iSazonov)
    added
    Resolution-FixedThe issue is fixed.The issue is fixed.
    on Apr 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

  • @rjmholt
    Rob Holt (rjmholt)

Labels

Issue-Code Cleanupthe issue is for cleaning up the code with no impact on functionalitythe issue is for cleaning up the code with no impact on functionalityResolution-FixedThe issue is fixed.The issue is fixed.

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions

    You are visitor 01402520 — 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.