Skip to content

added conditional processing for C# language features based on languageVersion / framework#3454

Merged
vlada-shubina merged 1 commit intodotnet:mainfrom
vlada-shubina:lang-version-issue
Jul 15, 2021
Merged

added conditional processing for C# language features based on languageVersion / framework#3454
vlada-shubina merged 1 commit intodotnet:mainfrom
vlada-shubina:lang-version-issue

Conversation

@vlada-shubina
Copy link
Copy Markdown
Member

@vlada-shubina vlada-shubina commented Jul 14, 2021

Problem

fixes #3449

Solution

Added conditional processing for classlib / console based on framework and language versions:

  • conditions do not take into account possible value of target-framework-override option (option is hidden from user)
  • conditions assume latest, default, latestMajor, preview language versions support all the features.

Checks:

  • Added unit tests

Copy link
Copy Markdown

@KathleenDollard KathleenDollard left a comment

Choose a reason for hiding this comment

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

I'll review later. Suggested a naming change.

The other comment does not require action at present.

<RootNamespace Condition="'$(name)' != '$(name{-VALUE-FORMS-}safe_namespace)'">Company.ClassLibrary1</RootNamespace>
<LangVersion Condition="'$(langVersion)' != ''">$(ProjectLanguageVersion)</LangVersion>
<Nullable Condition="('$(Framework)' != 'netstandard2.0')">enable</Nullable>
<DisableImplicitNamespaceImports Condition="'$(csharpFeature_GlobalUsings_Supported)' != 'true'">true</DisableImplicitNamespaceImports>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This was the other thing I was going to comment on. If we need this, it's a bug in the feature - if we aren't where implicit usings are supported, they should not need to be disabled.

However, let's get Preview 7 out working correctly (correctly being you can run templates and they then compile), so if this is needed today, please leave it in.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We need it when language version is < 10.0, otherwise it won't compile.

Copy link
Copy Markdown
Contributor

@DavidKarlas DavidKarlas left a comment

Choose a reason for hiding this comment

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

I liked idea of "additionalConfigFiles" a lot, assuming it could be shared between Library and Console templates, since that is not the case, I would prefer if we don't use it to give us more time to rethink what to do with this un-documented feature...


#endif
#if (!csharpFeature_TopLevelProgram)
namespace Company.ConsoleApplication1
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are you planning on duplicating this content for the file-scoped namespaces feature?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I haven't tested it yet, but I guess no. Top-level programs are supported since 9.0 and file scoped namespaces are supported since 10.0, so there should be no need as if top level program is not supported, file scoped namespaces won't be supported too.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ah of course, this is Program.cs 😄 👍

@vlada-shubina
Copy link
Copy Markdown
Member Author

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@vlada-shubina vlada-shubina reopened this Jul 15, 2021
@vlada-shubina vlada-shubina force-pushed the lang-version-issue branch 2 times, most recently from 36558ed to ba6184c Compare July 15, 2021 10:50
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 this pull request may close these issues.

Console and classlib cannot be built when langVersion is set to value less than 10 (net6.0)

5 participants