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

indent deeply-nested EQ_FORMALS correctly #546

Merged

Conversation

lorenzwalthert
Copy link
Collaborator

@lorenzwalthert lorenzwalthert commented Sep 22, 2019

Problem

non-technical: When nested deeper than 1 level, indention of function headers is generally not correct:

styler::style_text('MyClass <- R6::R6Class(
    "MyClass",
    public = list(initialize = function(my_arg,
                                        my_named_arg = 1) {
        return(invisible())
      }
    ),
  )')
#> MyClass <- R6::R6Class(
#>   "MyClass",
#>   public = list(initialize = function(my_arg,
#>                                         my_named_arg = 1) {
#>     return(invisible())
#>   }),
#> )

Created on 2019-09-23 by the reprex package (v0.3.0)

technical source: In apply_ref_indention_one(), the parse table is already flat, the indention of all tokens is absolute. However, for those where indention should be copied from another token, their indention must be relative, otherwise we'll double account for some indention.

See this exceprt from R/reindent.R, apply_ref_indention_one():

copied_spaces <- flattened_pd$col2[target_token]  # contains already absolute indention
shift <- copied_spaces
  flattened_pd$lag_spaces[token_to_update] <-
    flattened_pd$lag_spaces[token_to_update] + # contains also absolute indention
    shift
  # in sum, we have indention twice, hence, we must make sure that when flattened out,
  # the token that get their indention from another token contain relative indention.
 

Formals need to be aligned with the absolute indention of the opening brace. If you add another time their absolute indention, they will be indented way to much.

Solution

When flattening the parse table, keep the indention level of all tokens that have a ref_pos_id relative, so when added with an absolute indention in apply_ref_indention(), it will become absolute indention. Closes #541.

Problem: In apply_ref_indention_one(), the parse table is already flat, the indention of all tokens is absolute. However, for those where indention should be copied from another token, their indention must be relative, otherwise we'll double account for some indention. See tests: Formals need to be aligned with the absolute indention of the opening brace. If you add another time their absolute indention, they will be indented way to much.

Solution: When flattening the parse table, keep the indention level of all tokens that have a ref_pos_id relative, so when added with an absolute indention, it will become absolute indention.
@codecov-io
Copy link

@codecov-io codecov-io commented Sep 22, 2019

Codecov Report

Merging #546 into master will increase coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #546      +/-   ##
==========================================
+ Coverage   90.86%   90.88%   +0.02%     
==========================================
  Files          43       43              
  Lines        1806     1811       +5     
==========================================
+ Hits         1641     1646       +5     
  Misses        165      165
Impacted Files Coverage Δ
R/visit.R 98.43% <100%> (+0.13%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 69154c6...08382a8. Read the comment docs.

@lorenzwalthert lorenzwalthert merged commit dd1ccdd into r-lib:master Sep 24, 2019
4 checks passed
@lorenzwalthert lorenzwalthert deleted the no-linebreak-with-eq-formals branch Sep 24, 2019
@lorenzwalthert lorenzwalthert restored the no-linebreak-with-eq-formals branch Oct 16, 2019
@lorenzwalthert lorenzwalthert deleted the no-linebreak-with-eq-formals branch Jan 2, 2021
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.

2 participants