Skip to content
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.

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

Utilise NameOf and String Interpolation #2132

Closed
wants to merge 29 commits into from

Conversation

Projects
None yet
5 participants
@AdamSpeight2008
Copy link
Contributor

commented Apr 20, 2015

All of \Compilers\VisualBasic\Portable completed.

@bbarry

This comment has been minimized.

These aren't the same.

after      before  rightValue   leftValue ResultValue
-----      ------  ----------   --------- -----------
 True       False          -1          -1           1
 True       False           1          -1          -1
 True       False          -1           1          -1
 True       False           1           1           1

 True        True          -1          -1          -1
 True        True          -1          -1           0
False       False           0          -1          -1
False       False           0          -1           0
False       False           0          -1           1
 True        True           1          -1           0
 True        True           1          -1           1
 True        True          -1           0          -1
False       False          -1           0           0
 True        True          -1           0           1
False       False           0           0          -1
False       False           0           0           0
False       False           0           0           1
 True        True           1           0          -1
False       False           1           0           0
 True        True           1           0           1
 True        True          -1           1           0
 True        True          -1           1           1
False       False           0           1          -1
False       False           0           1           0
False       False           0           1           1
 True        True           1           1          -1
 True        True           1           1           0

AdamSpeight2008 added some commits Apr 15, 2015

@AdamSpeight2008

This comment has been minimized.

Copy link
Contributor Author

commented Apr 20, 2015

You can ignore @bbarry comments as I've kept the original Roslyn implementation

disposeMethodName & "(Of " & targetTypeName & ")(" & fieldName & ")" & vbCrLf &
"End Set" & vbCrLf
Return _
$"Set(ByVal {StringConstants.ValueParameterName} As {targetTypeName})

This comment has been minimized.

Copy link
@AdamSpeight2008

AdamSpeight2008 Apr 24, 2015

Author Contributor

Left aligned as I think the indentation, could cause issues due significant whitespace within the string literal,

This comment has been minimized.

Copy link
@gafter

gafter Jun 16, 2015

Member

Please preserve the indentation.

@@ -640,7 +640,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
''' </summary>
Friend Shadows Function WithPreviousSubmission(newPreviousSubmission As VisualBasicCompilation) As VisualBasicCompilation
If Not IsSubmission Then
Throw New NotSupportedException("Can't have a previousSubmission when not a submission")

This comment has been minimized.

Copy link
@gafter

gafter Jun 16, 2015

Member

This is just incorrect.

@@ -2192,9 +2191,9 @@ TryResync:
nameNew,
node.TextTokens.Node,
node.QuestionGreaterThanToken)
Else

This comment has been minimized.

Copy link
@gafter

gafter Jun 16, 2015

Member

No idea why you changed this.

@@ -2320,9 +2311,8 @@ TryResync:

If anyChanges Then
Return New XmlNameSyntax(node.Kind, node.GetDiagnostics, node.GetAnnotations, prefix, localName)
Else

This comment has been minimized.

Copy link
@gafter

gafter Jun 16, 2015

Member

No idea why you changed this.

@@ -2341,9 +2331,8 @@ TryResync:

If anyChanges Then
Return New XmlPrefixSyntax(node.Kind, node.GetDiagnostics, node.GetAnnotations, name, colon)
Else

This comment has been minimized.

Copy link
@gafter

gafter Jun 16, 2015

Member

No idea why you changed this.

GetMethodBlock(fieldName, MakeSafeName(_createOrDisposeMethod), targetTypeName) &
"End Property" & vbCrLf &
"End Class" & vbCrLf
Dim codeToParse As String = _

This comment has been minimized.

Copy link
@gafter

gafter Jun 16, 2015

Member

Would prefer some indentation in this string.

@@ -169,10 +170,12 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols
' return <backingField>
' End Get

Return "Get" & vbCrLf &

This comment has been minimized.

Copy link
@gafter

gafter Jun 16, 2015

Member

Indentation lost.

This comment has been minimized.

Copy link
@AdamSpeight2008

AdamSpeight2008 Jun 16, 2015

Author Contributor

@gafter String Literals have significant whitespace within their double quotes. If the multi-line string literal is indented, indentation is included within the string. Highly likely not what the user intended. See #2231 I think a comment explaining break indentation.

This comment has been minimized.

Copy link
@gafter

gafter Jun 16, 2015

Member

I'm referring to the indentation of the body of the Getter being indented relative to the Get and End Get statements.

@gafter gafter added this to the 1.1 milestone Jun 17, 2015

@gafter gafter self-assigned this Jun 17, 2015

AdamSpeight2008 AdamSpeight2008
[VB Compiler}
Remove some of extranious changes.
Adding Indentation into so to the contents of the string interpolations.
@gafter

This comment has been minimized.

Copy link
Member

commented Jun 23, 2015

29 commits is too much for a PR. Also, this PR no longer merges. Please address the current comments, bring it up-to-date, and squash to a single commit. Alternatively if you are no longer interested in offering these changes please close the PR.

@AdamSpeight2008

This comment has been minimized.

Copy link
Contributor Author

commented Jun 24, 2015

@gafter I'll resubmit when I get round to it, I'll use this just as a reference to locate the use cases.

@AdamSpeight2008 AdamSpeight2008 deleted the AdamSpeight2008:NameOf branch Oct 31, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.