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

Allowing line break transformers to depend on multi-line attribute #357

Closed
lorenzwalthert opened this issue Feb 24, 2018 · 12 comments
Closed

Comments

@lorenzwalthert
Copy link
Collaborator

lorenzwalthert commented Feb 24, 2018

Line break rules may depend on the structure of the parse table. So we may want to keep curly-brace-expressions as one-liner if the input was one line:

if (x) { x + (1+1) }

Note that this is not easily possible. The multi-line attribute is not up-to-date for the line break transformers, hence we cannot rely on it. Further, we cannot rely on line1 and line2, because this attribute was removed for safety beforehand (as it's also not up to date). We can try to "pre-reconstruct" the multi-line attribute by looking into the children, but that's brittle. One way out would be to split the line break transformers into two groups. One not depending on an up-to-date multi-line attribute and one dependent on it.
We could first apply the first transformer group, then update the multi-line attribute and then apply all transformers, updating the multi-line attribute between every transformer (which seems expensive).

@lorenzwalthert lorenzwalthert changed the title Allowing line break transformers depending on multiline attribute Allowing line break transformers to depend on multiline attribute Feb 24, 2018
@lorenzwalthert lorenzwalthert changed the title Allowing line break transformers to depend on multiline attribute Allowing line break transformers to depend on multi-line attribute Feb 24, 2018
@lorenzwalthert
Copy link
Collaborator Author

Reference: #342.

@lorenzwalthert
Copy link
Collaborator Author

This requires style guides to have a new structure:

create_style_guide(
    # transformer functions
    initialize                      = default_style_guide_attributes,
    line_break                      = line_break_manipulators,
    line_break_requiring_multi_line = new_transformers,  # new category
    space                           = space_manipulators,
    token                           = token_manipulators,
    indention                       = indention_modifier,
    # transformer options
    use_raw_indention               = use_raw_indention,
    reindention                     = reindention
  )

@lorenzwalthert
Copy link
Collaborator Author

lorenzwalthert commented Feb 26, 2018

@krlmlr what do you think? This is needed to solve #353 (comment) and #342 properly, which I wanted to be part of milestone styler 1.0.1 but it seems time consuming. I think especially #342 should be solved as part of styler 0.1.0, especially the case we can solve with an improper approach (i.e. without solving #357) and using a one-level deep multi-line attribute via lag_newlines.

@krlmlr
Copy link
Member

krlmlr commented Feb 26, 2018

I don't mind postponing #353, let's add a test for the behavior in #342 instead ;-)

@lorenzwalthert
Copy link
Collaborator Author

lorenzwalthert commented Feb 28, 2018

Blocks #351.

@lorenzwalthert
Copy link
Collaborator Author

lorenzwalthert commented Feb 28, 2018

Blocks #342.

@lorenzwalthert
Copy link
Collaborator Author

Blocks #371.

@lorenzwalthert
Copy link
Collaborator Author

Blocks #363.

@krlmlr
Copy link
Member

krlmlr commented Mar 21, 2018

Does the increasing number of "blocks" increase the priority of this issue?

@lorenzwalthert
Copy link
Collaborator Author

lorenzwalthert commented Mar 23, 2018

Not if a review of the issues yields that half of them are not blocked anymore 😊

@lorenzwalthert
Copy link
Collaborator Author

lorenzwalthert commented Oct 14, 2018

Blocks #428 (if we agree on @krlmlr's viewpoint 😄).

@lorenzwalthert
Copy link
Collaborator Author

It seems as a reasonable approach would indeed entail to add a new class of line-break transformers that require an up-to-date multi-line attribute as outlined above. This is potentially computatiaionally expensive.

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

No branches or pull requests

2 participants