Fix wrapping long strings logic in formatter

Author: spoonmilk

formatter/generic/genericformatter.cpp

@@ -183,6 +183,16 @@ struct Item
 			items.back().AddTokenToLastAtom(token);
 	}
 
+	void AddTokenToLastStringComponent(const InstructionTextToken& token)
+	{
+		if (!tokens.empty())
+			tokens.push_back(token);
+		else if (items.empty())
+			items.push_back(Item {StringComponent, {}, {token}, 0});
+		else
+			items.back().AddTokenToLastStringComponent(token);
+	}
+
 	void CalculateWidth()
 	{
 		width = 0;
@@ -204,7 +214,6 @@ struct ItemLayoutStackEntry
 	size_t desiredWidth;
 	size_t desiredContinuationWidth;
 	size_t desiredStringWidth;
-	size_t desiredStringContinuationWidth;
 	bool newLineOnReenteringScope;
 };
 
@@ -355,6 +364,16 @@ static vector<InstructionTextToken> ParseStringToken(
 
 static vector<Item> CreateStringGroups(const vector<Item>& items)
 {
+	// We handle strings mostly the same as other types except the introduction
+	// of the StringComponent and StringWhitespace types.
+	//
+	// The reason we introduce these is for the specific behaviors when formatting multiline
+	// string annotations. String annotations have a different desired width than tokens
+	// like arguments, comments, etc.
+	//
+	// Additionally, we don't wrap trailing whitespace until the preceding token is within
+	// the wrapping width, unlike other token types.
+
     vector<Item> result, pending;
     bool hasStrings = false;
     for (auto& i : items)
@@ -370,7 +389,7 @@ static vector<Item> CreateStringGroups(const vector<Item>& items)
 			else
 			{
 				for (auto& j : i.tokens)
-					pending.back().AddTokenToLastAtom(j);
+					pending.back().AddTokenToLastStringComponent(j);
 				result.push_back(Item {StringComponent, pending, {}, 0});
 			}
 			pending.clear();
@@ -385,18 +404,18 @@ static vector<Item> CreateStringGroups(const vector<Item>& items)
     			result.push_back(Item {StringComponent, pending, {}, 0});
     			pending.clear();
     		}
-    		result.push_back(Item {StringWhitespace, i.items, i.tokens, i.width});
+    		result.push_back(Item {StringWhitespace, i.items, i.tokens, 0});
     	}
     	else if (i.type == FormatSpecifier || i.type == EscapeSequence)
     	{
     		// Flush previous tokens before special sequences like format specifiers or
     		// escape sequences
     		if (!pending.empty())
     		{
-    			result.push_back(Item {StringComponent, pending, {}, 0 });
+    			result.push_back(Item {StringComponent, pending, {}, 0});
     			pending.clear();
     		}
-    		result.push_back(Item { StringComponent, i.items, i.tokens, i.width});
+    		result.push_back(Item { StringComponent, i.items, i.tokens, 0});
     	}
     	else if (i.type == StartOfContainer && pending.empty())
     	{
@@ -750,19 +769,10 @@ vector<DisassemblyTextLine> GenericLineFormatter::FormatLines(
 		if (indentation < settings.desiredLineLength)
 		{
 			size_t remainingStringWidth = settings.desiredLineLength - indentation;
-			if (remainingStringWidth > desiredWidth)
+			if (remainingStringWidth > desiredStringWidth)
 				desiredStringWidth = remainingStringWidth;
 		}
 
-		// Compute target width for continuation string wrapping lines
-		size_t desiredStringContinuationWidth = settings.stringWrappingWidth;
-		if (continuationIndentation < settings.desiredLineLength)
-		{
-			size_t remainingStringWidth = settings.desiredLineLength - continuationIndentation;
-			if (remainingStringWidth > desiredStringContinuationWidth)
-				desiredStringContinuationWidth = remainingStringWidth;
-		}
-
 		// Gather the indentation tokens at the beginning of the line
 		vector<InstructionTextToken> indentationTokens = currentLine.GetAddressAndIndentationTokens();
 		size_t tokenIndex = indentationTokens.size();
@@ -927,7 +937,7 @@ vector<DisassemblyTextLine> GenericLineFormatter::FormatLines(
 		bool firstTokenOfLine = true;
 
 		stack<ItemLayoutStackEntry> layoutStack;
-		layoutStack.push({items, additionalContinuationIndentation, desiredWidth, desiredContinuationWidth, desiredStringWidth, desiredStringContinuationWidth, false});
+		layoutStack.push({items, additionalContinuationIndentation, desiredWidth, desiredContinuationWidth, desiredStringWidth, false});
 
 		auto newLine = [&]() {
 			if (!firstTokenOfLine)
@@ -952,7 +962,6 @@ vector<DisassemblyTextLine> GenericLineFormatter::FormatLines(
 			outputLine.tokens.emplace_back(TextToken, string(additionalContinuationIndentation, ' '));
 			currentWidth = 0;
 			desiredWidth = desiredContinuationWidth;
-			desiredStringWidth = desiredStringContinuationWidth;
 			firstTokenOfLine = true;
 		};
 
@@ -966,7 +975,6 @@ vector<DisassemblyTextLine> GenericLineFormatter::FormatLines(
 			desiredWidth = layoutStackEntry.desiredWidth;
 			desiredContinuationWidth = layoutStackEntry.desiredContinuationWidth;
 			desiredStringWidth = layoutStackEntry.desiredStringWidth;
-			desiredStringContinuationWidth = layoutStackEntry.desiredStringContinuationWidth;
 
 			// Check to see if the scope we are returning to needs a new line. This is used when an argument
 			// spans multiple lines. The rest of the arguments are placed on separate lines from the long argument.
@@ -979,10 +987,11 @@ vector<DisassemblyTextLine> GenericLineFormatter::FormatLines(
 				{
 					// If a string is too wide to fit on the current line, create a newline
 					// without additional indentation
+					LogError("Line length vs. desired %zu vs %zu", currentWidth + item->width, desiredStringWidth);
 					newLine();
 					continue;
 				}
-				if (currentWidth + item->width > desiredWidth && item->type != StringWhitespace)
+				if (currentWidth + item->width > desiredWidth && item->type != StringWhitespace && item->type != StringComponent)
 				{
 					// Current item is too wide to fit on the current line, will need to start a new line.
 					// Whitespace is allowed to be too wide; we push it on as the preceding word is wrapped.
@@ -1001,7 +1010,7 @@ vector<DisassemblyTextLine> GenericLineFormatter::FormatLines(
 							if (next != items.end())
 							{
 								layoutStack.push({vector(next, items.end()), additionalContinuationIndentation,
-									desiredWidth, desiredContinuationWidth, desiredStringWidth, desiredStringContinuationWidth, true});
+									desiredWidth, desiredContinuationWidth, desiredStringWidth, true});
 							}
 
 							newLine();
@@ -1012,13 +1021,8 @@ vector<DisassemblyTextLine> GenericLineFormatter::FormatLines(
 							else
 								desiredContinuationWidth -= settings.tabWidth;
 
-							if (desiredStringContinuationWidth < settings.minimumContentLength + settings.tabWidth)
-								desiredStringContinuationWidth = settings.minimumContentLength;
-							else
-								desiredStringContinuationWidth -= settings.tabWidth;
-
 							layoutStack.push({item->items, additionalContinuationIndentation, desiredWidth,
-								desiredContinuationWidth, desiredStringWidth, desiredStringContinuationWidth, false});
+								desiredContinuationWidth, desiredStringWidth, false});
 							break;
 						}
 
@@ -1028,10 +1032,10 @@ vector<DisassemblyTextLine> GenericLineFormatter::FormatLines(
 							if (next != items.end())
 							{
 								layoutStack.push({vector(next, items.end()), additionalContinuationIndentation,
-									desiredWidth, desiredContinuationWidth, desiredStringWidth, desiredStringContinuationWidth, false});
+									desiredWidth, desiredContinuationWidth, desiredStringWidth, false});
 							}
 							layoutStack.push({item->items, additionalContinuationIndentation, desiredWidth,
-								desiredContinuationWidth, desiredStringWidth, desiredStringContinuationWidth, false});
+								desiredContinuationWidth, desiredStringWidth,  false});
 							break;
 						}