Support language-server arguments in the extension.

[jonmeow]

I was on the fence about just having a string which was "language-server", but was thinking split options might be less error-prone (e.g., changing options to just "-v" and trying to figure out why nothing worked).

[zygoloid]

I think this converts --foo="bar baz" to two arguments: --foo="bar and baz", because the regex will greedily pick the longest match starting with the -; that seems surprising to me. If we exclude ' and " from the \S, we'd get --foo= and bar baz, which still seems surprising to me but is closer to what I'd expect.

Maybe we could match "([^"]*)"|'([^']*)'|([^'"\s]+)|(\s+), and append to the current argument in the first three cases and start a new argument in the fourth case?

[jonmeow]

Note for --foo="bar baz (no closing quote), the suggestion would fail [silently, discarding args]. In the current code, it falls under the \S case -- but if you keep the \S case, your concatenation approach won't work.

I understand it's not perfect, but I'm also not familiar enough with typescript to fix this quickly. Let me know if you'd prefer I close the PR, or if you can suggest code.

[zygoloid]

OK, maybe just add a TODO for now, then.

Is there any reasonable way to add a test for this?

[jonmeow]

OK, maybe just add a TODO for now, then.

Ended up spending enough time on this that I just figured out a different approach which should work.

Is there any reasonable way to add a test for this?

Bazel doesn't seem to be prioritizing js/ts support, and I'm not sure what your "reasonable" threshold is.

There's https://github.com/aspect-build/rules_js and https://github.com/aspect-build/rules_ts, but they're not well-aimed at our use-case of having just a little ts buried in the repo here and there. I've considered investing time into figuring that out, have occasionally banged my head against bazel for a couple hours trying to figure out a good setup (now including today), but so far I've instead tried to keep the amount of JavaScript/TypeScript as niche as possible (now including today). My guess is that with a couple days I could probably figure out something to have a unit test for this code -- but it's not trivial, and I suspect it'd be fragile. I continue to hope that someone more fluent in ts and bazel will come along and clean up.

TBH if I could find an easy library to parse the args, I would. But quick searches were turning up advice on how to just write it myself.

[zygoloid]

Suggested change

	function splitQuotedString(argsString: string): string[] {
	const args: string[] = [];
	let arg = '';
	let inSingleQuotes = false;
	let inDoubleQuotes = false;
	let escaped = false;
	while ((match = regex.exec(carbonArgs)) !== null) {
	if (match[1]) {
	result.push(match[1]);
	} else if (match[2]) {
	result.push(match[2]);
	} else if (match[3]) {
	result.push(match[3]);
	for (const char of argsString) {
	if (escaped) {
	arg += char;
	escaped = false;
	continue;
	}
	switch (char) {
	case '\\':
	escaped = true;
	continue;
	case "'":
	if (!inDoubleQuotes) {
	inSingleQuotes = !inSingleQuotes;
	continue;
	}
	break;
	case '"':
	if (!inSingleQuotes) {
	inDoubleQuotes = !inDoubleQuotes;
	continue;
	}
	break;
	case ' ':
	if (!inSingleQuotes && !inDoubleQuotes) {
	args.push(arg);
	arg = '';
	}
	break;
	}
	arg += char;
	}
	return result;
	if (arg.length > 0) {
	args.push(arg);
	}
	return args;
	}
	function splitQuotedString(argsString: string): string[] {
	const args: string[] = [];
	let arg = '';
	let inSingleQuotes = false;
	let inDoubleQuotes = false;
	let escaped = false;
	let empty = true;
	for (const char of argsString) {
	const was_empty = empty;
	empty = false;
	if (escaped) {
	arg += char;
	escaped = false;
	continue;
	}
	switch (char) {
	case '\\':
	escaped = true;
	continue;
	case "'":
	if (!inDoubleQuotes) {
	inSingleQuotes = !inSingleQuotes;
	continue;
	}
	break;
	case '"':
	if (!inSingleQuotes) {
	inDoubleQuotes = !inDoubleQuotes;
	continue;
	}
	break;
	case ' ':
	if (!inSingleQuotes && !inDoubleQuotes) {
	if (!was_empty) {
	args.push(arg);
	arg = '';
	}
	empty = true;
	continue;
	}
	break;
	}
	arg += char;
	}
	if (!empty) {
	args.push(arg);
	}
	return args;
	}

Some tweaks to handle "" as an argument, multiple spaces between arguments, and an argument list ending in a space.

[jonmeow]

Adopting this, but in the future I'd find smaller deltas more helpful. The delta is broken and it's difficult to actually understand the changes here without applying them.

[jonmeow]

Note, part of the issue here I think is you're [manually] making a suggestion around code that I assume GitHub was not happy to take a suggestion for.

[zygoloid]

Yeah, github's suggestion feature is a bit broken, unfortunately. I'll try to split the suggestion up into chunks in future.