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

Fix auto indent after end #2086

Merged
merged 2 commits into from
May 27, 2024

Conversation

thomasmarshall
Copy link
Contributor

@thomasmarshall thomasmarshall commented May 23, 2024

Motivation

I noticed a few small issues since enabling on-type formatting recently. In particular, it adds incorrect indentation in some cases after typing the end keyword. It's not a big deal really because the code will be auto-formatted when saving the file, but I thought I would try to fix the easier ones to make the editing experience a bit nicer.

Complex body indentation

Before After

Existing indentation

Before After

Implementation

  • For the complex body indentation, it loops through each line of each node to add indentation as nodes can span multiple lines.
  • For existing indentation, it finds the indentation correctly for comparison.

Automated Tests

I've added a test case for each fix to OnTypeFormattingTest.

Manual Tests

if true
if true
  true
end
en

Then add the final d.

if true
  true
en

Then add the final d.

This commit fixes the auto indentation after the `end` keyword when the
body is more complex, such as when there are nested blocks.

Previously the following code:

```ruby
if foo
if bar
  baz
end
end
```

…would have been indented like this:

```rb
if foo
  if bar
  baz
end
end
```

Now it will be correctly indented like this:

```rb
if foo
  if bar
    baz
  end
end
```

It does so by indenting each line in the node location, instead of just
the first one.
This commit ensures auto-indentation after an `end` keyword does not add
extra indentation if the body is already indented.

Previously the following code:

```rb
if foo
  bar
end
```

…would have been indented like this:

```rb
if foo
    bar
end
```

Now it will be correctly indented like this:

```rb
if foo
  bar
end
```

It does so by finding the correct current indentation level, and
skipping indentation if it already matches.
@thomasmarshall thomasmarshall requested a review from a team as a code owner May 23, 2024 19:19
@vinistock vinistock added bugfix This PR will fix an existing bug server This pull request should be included in the server gem's release notes labels May 27, 2024
Copy link
Member

@vinistock vinistock left a comment

Choose a reason for hiding this comment

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

Thank you for the fix!

@vinistock vinistock merged commit d3cc19b into Shopify:main May 27, 2024
19 of 20 checks passed
andyw8 pushed a commit that referenced this pull request May 29, 2024
* Fix complex body indentation after end keyword

This commit fixes the auto indentation after the `end` keyword when the
body is more complex, such as when there are nested blocks.

Previously the following code:

```ruby
if foo
if bar
  baz
end
end
```

…would have been indented like this:

```rb
if foo
  if bar
  baz
end
end
```

Now it will be correctly indented like this:

```rb
if foo
  if bar
    baz
  end
end
```

It does so by indenting each line in the node location, instead of just
the first one.

* Fix extra indentation after end keyword

This commit ensures auto-indentation after an `end` keyword does not add
extra indentation if the body is already indented.

Previously the following code:

```rb
if foo
  bar
end
```

…would have been indented like this:

```rb
if foo
    bar
end
```

Now it will be correctly indented like this:

```rb
if foo
  bar
end
```

It does so by finding the correct current indentation level, and
skipping indentation if it already matches.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This PR will fix an existing bug server This pull request should be included in the server gem's release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants