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 range formatting handling initial indentation #268

Merged
merged 2 commits into from
Jun 1, 2020

Conversation

renkun-ken
Copy link
Member

@renkun-ken renkun-ken commented Jun 1, 2020

Closes #264.

If the selection range has non-zero indentation, then a apply_initial_indention function injected to the formatting styles so that the top level expressions get the indentation.

For example, to format the code inside the the following function

my_fun <- function() {
  data <- query(
    con ,
    "select name, group, max(update_time) as max_time
from users
where date = ?date
group by group",
date = 20200601
  )
x = 2
y = 3  
}

the code will be enclosed inside { ... } and then apply_initial_indention will apply the indention to all top-level expressions of the selection, then the curly braces are droped to get the formatted text.

The result is like

my_fun <- function() {
  data <- query(
    con,
    "select name, group, max(update_time) as max_time
from users
where date = ?date
group by group",
    date = 20200601
  )
  x = 2
  y = 3
}

@@ -106,7 +122,7 @@ range_formatting_reply <- function(id, uri, document, range, options) {
}

selection <- document$content[(row1:row2) + 1]
indentation <- stringi::stri_extract_first_regex(selection[1], "^\\s*")
indentation <- nchar(stringi::stri_extract_first_regex(selection[1], "^\\s*"))
new_text <- style_text(selection, style, indentation = indentation)
Copy link
Member

Choose a reason for hiding this comment

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

it doesn't work with tabs unfortunately. Though it seems that styler doesn't support tabs r-lib/styler#347

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it does not support tabs. So I just ignore this case unless somebody complains.

Copy link
Member

@randy3k randy3k left a comment

Choose a reason for hiding this comment

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

LGTM. Please merge after the tests pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Range formatting always adds indention when the range has initial indention
2 participants