Increase available span for 'Convert to interpolated string' refactoring #30356
Conversation
| var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); | ||
|
|
||
| // The string literal has to at least be contained in a concatenation of some form. | ||
| // i.e. "goo" + a or a + "goo". However, those concats could be in larger | ||
| // concats as well. Walk to the top of that entire chain. | ||
|
|
||
| var literalExpression = token.Parent; |
There was a problem hiding this comment.
this is not named property anymore.
There was a problem hiding this comment.
I updated the comment.
| { | ||
| return; | ||
| } | ||
|
|
||
| var cancellationToken = context.CancellationToken; |
There was a problem hiding this comment.
overall i like this change. however, my preference is:
if we allow non-empty spans, we require that the span start/end can't be in the middle of a token. don't want someone making a random textual selection, but then getting these sorts of features. it feels pretty confusing as to what the selection/refactoring then mean IMO.
There was a problem hiding this comment.
@CyrusNajmabadi I understand your preference, however, I chose to do it this way because the other 'Convert to interpolated string' refactoring works the same way. If they behave differently (as they were doing before this change), that's also confusing.
I.e. when you have:
string.Format("Hello {0} there {1}", 1, 2);
you can also have a selection, either in full or in part, and the refactoring will be offered.
I'd be happy to change it back to not allowing selections if you prefer it that way though. Please let me know.
There was a problem hiding this comment.
My personal preference is that we should continually tring to make things better, even if that leads to periods of inconsistency. :) I personally believe it would be better in both cases to not support this functionality if there is a selection that splits individual tokens. However, that doesn't mean i don't want to fix one case just because another case exists. Always be improving, and we'll continually reach better and better places. If we dont' improve things because of other issues elsewhere, we commonly reach local minimums :-/
There was a problem hiding this comment.
@CyrusNajmabadi You don't need to convince me, I completely agree! Just wanted to make sure you knew the difference in the similar refactoring :). I can't make the changes today anymore, I hope I can get around to it tomorrow.
There was a problem hiding this comment.
I disallowed selections again now.
jinujoseph
added
Area-IDE
Community
|
retest this please |
| var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false); | ||
|
|
||
| // The string literal has to at least be contained in a concatenation of some form. | ||
| // i.e. "goo" + a or a + "goo". However, those concats could be in larger | ||
| // concats as well. Walk to the top of that entire chain. | ||
|
|
||
| var literalExpression = token.Parent; |
There was a problem hiding this comment.
nit: this now can be something other than a literal expression, maybe change the variable name to to selectedExpression. Similarly for the comment above, e.g. change "The string literal" to "The selected token"
There was a problem hiding this comment.
Thanks for the suggestion, I have made this change.
I also rebased on master again.
…using when selecting part of to be interpolated string
rik-smeets
deleted the
increase-span-convert-to-interpolated-string-refactoring
branch
Resolves #16981.
The available span where the 'Convert to interpolated string' refactoring offered is widened. The available span is now similar to the refactoring on a
string.Format()statement.For example (using the example from the referenced issue):
Console.WriteLine(3 + "Hi" + 3 + "There");Before this change, the refactoring was only offered when the caret was touching the "Hi" or "There" strings (including quotation marks). Now, it is also offered on the non-string tokens (i.e.
3and5) and the plus tokens (or ampersand in VB). Lastly, it is offered when having a selection of either part of or the entire to be interpolated string.