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

Wrapping of concatenated string literals #495

Open
Gama11 opened this issue Jun 8, 2019 · 3 comments
Open

Wrapping of concatenated string literals #495

Gama11 opened this issue Jun 8, 2019 · 3 comments
Labels
enhancement New feature or request wrapping Incorrect or undesirable wrapping

Comments

@Gama11
Copy link
Member

Gama11 commented Jun 8, 2019

Describe the bug

Another one from the vshaxe diff since 1.7.1:

Input file

class Main {
	static function main() {
		context.sendShowMessage(Error, 'Invalid compiler argument \'$option\' detected. '
			+ 'Please verify "haxe.configurations" and "haxe.displayServer.arguments".');
	}
}

Broken output

class Main {
	static function main() {
		context.sendShowMessage(Error,
			'Invalid compiler argument \'$option\' detected. ' + 'Please verify "haxe.configurations" and "haxe.displayServer.arguments".');
	}
}

Not sure if this is technically a bug, but the wrapping in the "before" snippet makes a lot more sense to me.

@Gama11 Gama11 added the wrapping Incorrect or undesirable wrapping label Jun 8, 2019
@AlexHaxe AlexHaxe added the enhancement New feature or request label Jun 8, 2019
@AlexHaxe AlexHaxe added this to the Wrapping code generation 5 milestone Jun 8, 2019
@AlexHaxe
Copy link
Member

AlexHaxe commented Jun 8, 2019

There are two wrapping rules that could apply to that line: call parameter and OpAdd chain, for now the first one wins.

Wrapping code currently doesn't have a weighting function or even a way to preview or play with different combinations of wrapping rules.

Creating a weighting function involves figuring out how to score different wrapping outcomes, so that "before" has a higher score than "broken".

@Gama11
Copy link
Member Author

Gama11 commented Jun 8, 2019

Should a single + already count as an OpAdd "chain"?

@AlexHaxe
Copy link
Member

AlexHaxe commented Jun 8, 2019

Well, + already implies two items.
And despite it being a valid OpAdd chain or not, wrapping at + and wrapping at , both produce a valid output below max line length.
So either way there needs to be some scoring for formatter to decide which wrapping to apply on a particular line that has two or more competing wrapping rules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request wrapping Incorrect or undesirable wrapping
Projects
None yet
Development

No branches or pull requests

2 participants