Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

Port and test VB Information, DoubleType, DecimalType, Versioned #31252

Merged
merged 5 commits into from
Jul 25, 2018
Merged

Port and test VB Information, DoubleType, DecimalType, Versioned #31252

merged 5 commits into from
Jul 25, 2018

Conversation

bbowyersmyth
Copy link
Contributor

Contributes to https://github.com/dotnet/corefx/issues/31181

Partial port of Information.vb and Versioned.vb. Remaining functions do COM lookups which require extra attention.

Information.IsNumeric with char array bug recorded as a test case.


#Region " BACKWARDS COMPATIBILITY. These functions (IsNumeric, TypeName, SystemTypeName, VbTypeName) have been superceded by the versions in Versioned.vb "

'WARNING WARNING WARNING WARNING WARNING
Copy link
Member

Choose a reason for hiding this comment

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

I think this warning can be removed (Everett = VS2003, Orcas = VS2005...)

Also the region markers can go - there is no need to record compat with VS2002. Also I notice that in MSDN, Versioned.IsNumeric is marked not to use, so maybe it's not so superseded.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had never heard of the Versioned namespace. What appears to happen is that the VB compiler silently moves calls to Information.IsNumeric to Versioned.IsNumeric. So the Information variants exist for binary backwards compatibility (or calls from other languages).


#Region " BACKWARDS COMPATIBILITY "

'WARNING WARNING WARNING WARNING WARNING
Copy link
Member

Choose a reason for hiding this comment

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

I would remove all these compat warning comments and #region markers -- they are 15 years old at this point.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed

@danmoseley
Copy link
Member

Thanks for doing this @bbowyersmyth

@danmoseley
Copy link
Member

@OmarTawfik any other feedback?


#Region " BACKWARDS COMPATIBILITY "

'WARNING WARNING WARNING WARNING WARNING
Copy link
Member

Choose a reason for hiding this comment

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

Another to remove - I would also remove #Region " BACKWARDS COMPATIBILITY "

@danmoseley
Copy link
Member

@bbowyersmyth could you please rebase this one also?

Public Shared Function ToDate(Value As String) As Date
Dim parsedDate As Global.System.DateTime
Const parseStyle As Global.System.Globalization.DateTimeStyles =
Global.System.Globalization.DateTimeStyles.AllowWhiteSpaces Or
Global.System.Globalization.DateTimeStyles.NoCurrentDateDefault
Dim culture As Global.System.Globalization.CultureInfo = GetCultureInfo()
Dim result As Boolean = Global.System.DateTime.TryParse(ToHalfwidthNumbers(Value, culture), culture, parseStyle, parsedDate)
Dim result As Boolean = Global.System.DateTime.TryParse(ToHalfwidthNumbers(value, culture), culture, parseStyle, parsedDate)
Copy link
Contributor

Choose a reason for hiding this comment

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

value [](start = 87, length = 5)

Unintended change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Changed the casing back.

Copy link
Contributor

@OmarTawfik OmarTawfik left a comment

Choose a reason for hiding this comment

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

Thanks for contributing!
left one comment.

@OmarTawfik
Copy link
Contributor

@danmosemsft do we have a work item logged somewhere to compare the added public API surface to desktop? With all of these changes (specially with how sometimes editors automatically lowercase things that should not (like parameter names)) it might be a breaking change to ship them this way.

@danmoseley
Copy link
Member

@dotnet-bot test Linux arm release build

@danmoseley
Copy link
Member

@OmarTawfik feel free to open one but to me that is a necessary step before closing the matter tracking issue. It is very easy with our API tooling

@danmoseley danmoseley merged commit 5f635b2 into dotnet:master Jul 25, 2018
@danmoseley
Copy link
Member

Thanks @bbowyersmyth

@bbowyersmyth bbowyersmyth deleted the VB_Information branch July 25, 2018 20:32
@karelz karelz added this to the 3.0 milestone Aug 21, 2018
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
…net/corefx#31252)

* Port from referencesource

* Cleanup and tests

* Remove parameter name validation

* Address PR feedback

* Address PR feedback


Commit migrated from dotnet/corefx@5f635b2
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants