Prevent incorrect indentation for Ruby's in and when keywords

[vinistock]

Currently, the language configuration decreases indentation immediately after the words in or when are typed. However, that produces some incorrect decreases.

For example, if you are including a module, the include keyword gets incorrectly dedented back.

class Foo
  include Bar # <<< indentation here is decreased incorrectly, since it's not an `in` keyword
end

The when keyword has similar issues, since you can create methods named when_something_happens and then invoking the method gets dedented incorrectly.

If we change the regex to wait for one blank space after both in and when keywords, then the indentation decrease works properly - since the blank spaces after both of them is required to specify the branch condition.

[jscheel]

This happens with end as well. While end makes sense to decrement indention, it isn't always the keyword. IIRC, this indention happened later in the past, because I used to be able to type variable names like enduser without the decremented indention auto-triggering.

[vinistock]

@jscheel Yeah, we should fix that as well, although I'll split it in a separate PR.

For the end tokens, I think it would make sense to only dedent after a line break, since it's not usual to add a space after the end.

[vinistock]

@rebornix any concerns with this PR?

[rebornix]

@vinistock thanks for your contribution. It looks great to me, I'd like to test this thoroughly (maybe we could add some tests for it) before merging the PR. Really appreciate your help.

[vinistock]

I'm happy to add tests for it. Is there an example test for these language configuration files that I can base it on?

[rebornix]

@vinistock it would be great if we can have tests like

vscode/src/vs/editor/contrib/indentation/test/browser/indentation.test.ts

Lines 289 to 305 in b9511a9

	test('issue #167299: Blank line removes indent', () => {
	const languageId = 'blankLineRemovesIndent';
	const model = createTextModel("", languageId, {});
	disposables.add(model);
	withTestCodeEditor(model, { autoIndent: 'full' }, (editor, viewModel, instantiationService) => {
	const languageService = instantiationService.get(ILanguageService);
	const languageConfigurationService = instantiationService.get(ILanguageConfigurationService);
	disposables.add(languageService.registerLanguage({ id: languageId }));
	disposables.add(languageConfigurationService.register(languageId, {
	brackets: [
	['{', '}'],
	['[', ']'],
	['(', ')']
	],
	indentationRules: javascriptIndentationRules,
	onEnterRules: javascriptOnEnterRules
	}));

but I can push to your branch to add some tests if you prefer, either works for me.

[vinistock]

@rebornix I put together a test, but I'm having trouble with my local setup. I can't seem to yarn install successfully and thus can't run tests to verify they actually work.

If you don't mind, I'd appreciate you pushing the tests to this branch. This is what I had:

      // Other tests
      // ...

      suite('Editor Contrib - Auto Dedent On Type', () => {
	let disposables: DisposableStore;

	setup(() => {
		disposables = new DisposableStore();
	});

	teardown(() => {
		disposables.dispose();
	});

	ensureNoDisposablesAreLeakedInTestSuite();

	test('issue #198350: in or when incorrectly match non keywords for Ruby', () => {
		const languageId = "ruby";
		const model = createTextModel("", languageId, {});
		disposables.add(model);

		withTestCodeEditor(model, { autoIndent: "full" }, (editor, viewModel, instantiationService) => {
			const languageService = instantiationService.get(ILanguageService);
			const languageConfigurationService = instantiationService.get(ILanguageConfigurationService);
			disposables.add(languageService.registerLanguage({ id: languageId }));
			disposables.add(languageConfigurationService.register(languageId, {
				brackets: [
					['{', '}'],
					['[', ']'],
					['(', ')']
				],
				indentationRules: rubyIndentationRules,
			}));


			viewModel.type("def foo\n  in");
			assert.strictEqual(model.getValue(), "def foo\n  in");
		});
	});

[rebornix]

@vinistock it seems there are issues with the branch that our CI failed to run properly. I cherry picked your commit and packed in another PR #205397. Let's track it over there. Thanks again for your contribution!

[rebornix]

@vinistock your change is merged via #205397 and should be available in next Insiders, cheers!

[vinistock]

Thank you @rebornix!

[jscheel]

@vinistock @rebornix I'm still thinking about end causing unexpected dedents as well, and what can be done about that. It seems that the core problem is that this is relying on a pure regex to determine if auto indent/dedent should occur. However, this can only be an approximation, as we need to know what the next character entered is. Simply looking for a word boundary means that typing something as basic as enduser will immediately trigger a dedent. The determination cannot actually be made until the user types the next character. If it is a space or a carriage return, then yes, dedent should happen. But just checking on \b will cause it to trigger regardless, unless you are typing on the last line.

[vinistock]

Yeah, I agree we should fix it. I believe matching on line breaks would make the most sense, since that's when you want to have it dedented.

I made an attempt, but it doesn't seem to be working. I suspect it's something with the line break regex. @rebornix do you see anything obvious that might be wrong in the new regex? Does VS Code accept \r?\n as a line break matcher?

[rebornix]

@vinistock the regular expression was validated against the line content, which excludes the line breaks, thus it won't match. That's what we have for the whole regex based system, it would be a huge change if we change that. Regex system is never perfect and I personally believe a "range" formatter is always a better fit.

[vinistock]

Thanks for the context. If we can't match on line breaks, then I don't think it's possible to fix this without on type formatting indeed. We need to add something to the Ruby LSP to correct format the expression after VS Code's regular dedent.