Skip to content
This repository was archived by the owner on Apr 14, 2022. It is now read-only.

Fix string concatenation spacing in line formatter #144

Merged
jakebailey merged 1 commit into from
Sep 27, 2018
Merged

Fix string concatenation spacing in line formatter #144

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions src/Analysis/Engine/Test/LineFormatterTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,19 @@ public void CommentAfterOperator() {
AssertSingleLineFormat("a+# comment\nb", "a + # comment");
}

[DataRow("'a''b'", "'a' 'b'")]
[DataRow("'a' 'b'", "'a' 'b'")]
[DataRow("'''a''''''b'''", "'''a''' '''b'''")]
[DataRow("'''a'''r'''b'''", "'''a''' r'''b'''")]
[DataRow("\"a\"\"b\"", "\"a\" \"b\"")]
[DataRow("\"a\" \"b\"", "\"a\" \"b\"")]
[DataRow("\"\"\"a\"\"\"\"\"\"b\"\"\"", "\"\"\"a\"\"\" \"\"\"b\"\"\"")]
[DataRow("\"\"\"a\"\"\"r\"\"\"b\"\"\"", "\"\"\"a\"\"\" r\"\"\"b\"\"\"")]
[DataTestMethod, Priority(0)]
public void StringConcat(string code, string expected) {
AssertSingleLineFormat(code, expected);
}


[TestMethod, Priority(0)]
public void GrammarFile() {
Expand Down
23 changes: 10 additions & 13 deletions src/LanguageServer/Impl/Implementation/LineFormatter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,8 +287,15 @@ public TextEdit[] FormatLine(int line) {
builder.EnsureEndsWithSpace();
break;

case TokenKind.Name:
case TokenKind.Constant:
if (token.IsString && next != null && next.IsString) {
Copy link
Copy Markdown
Contributor

@AlexanderSher AlexanderSher Sep 26, 2018

Choose a reason for hiding this comment

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

Does next == null means that it is the last token?
Also, do you need to check if they are on the same line?

Copy link
Copy Markdown
Member Author

@jakebailey jakebailey Sep 26, 2018

Choose a reason for hiding this comment

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

Yes, that next would be null if the current token were the last token on the line. That next is pulled from the list of tokens of the line, not the linked-list that's happening in TokenExt which could peer forward. I wouldn't want to look at the next line, since even if the next token (ignoring whitespace) happens to be a string literal, the fact that it's on another line implies that there's already spacing and adding a space here would only be adding trailing whitespace (which would then be trimmed away).

builder.Append(token);
builder.EnsureEndsWithSpace();
break;
}
goto case TokenKind.Name;

case TokenKind.Name:
case TokenKind.KeywordFalse:
case TokenKind.KeywordTrue:
case TokenKind.Ellipsis: // Ellipsis is a value
Expand Down Expand Up @@ -410,19 +417,9 @@ public bool MatchesClose(TokenExt other) {

public bool IsKeyword => (Kind >= TokenKind.FirstKeyword && Kind <= TokenKind.LastKeyword) || Kind == TokenKind.KeywordAsync || Kind == TokenKind.KeywordAwait;

public bool IsMultilineString {
get {
if (Span.Start.Line == Span.End.Line) {
return false;
}
public bool IsString => Kind == TokenKind.Constant && Token != Tokens.NoneToken && (Token.Value is string || Token.Value is AsciiString);

if (Kind != TokenKind.Constant || Token == Tokens.NoneToken) {
return false;
}

return Token.Value is string || Token.Value is AsciiString;
}
}
public bool IsMultilineString => Span.Start.Line != Span.End.Line && IsString;

public bool IsSimpleSliceToLeft {
get {
Expand Down