Abstract stylist to libcst style conversion #4749
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MichaReiser
approved these changes
May 31, 2023
PR Check ResultsEcosystem✅ ecosystem check detected no changes. BenchmarkLinuxWindows |
i already though about making this a free standing function in the ruff crate otherwise, @charliermarsh any preferences? |
|
It wasn't intentional but it is a heavy dependency. Should we make it a feature? |
konstin
added a commit
that referenced
this pull request
Jun 1, 2023
Follow-up/alternative to #4749 we can either merge into it or discard.
|
Current dependencies on/for this PR:
This comment was auto-generated by Graphite. |
charliermarsh
approved these changes
Jun 1, 2023
crates/ruff/src/autofix/codemods.rs
Outdated
| /// Given an import statement, remove any imports that are specified in the `imports` iterator. | ||
| /// | ||
| /// Returns `Ok(None)` if the statement is empty after removing the imports. | ||
| pub(crate) fn remove_imports<'a>( |
There was a problem hiding this comment.
Does this just need a rebase? Why was this deleted?
There was a problem hiding this comment.
i think the rebase went wrong, i have to fix that
|
Found a better solution that abstracts away more code |
|
Looks reasonable to me. |
MichaReiser
approved these changes
Jun 5, 2023
crates/ruff/src/autofix/codemods.rs
Outdated
| @@ -11,6 +11,23 @@ use ruff_python_ast::source_code::{Locator, Stylist}; | |||
| use crate::cst::helpers::compose_module_path; | |||
| use crate::cst::matchers::match_statement; | |||
|
|
|||
| /// Glue code to make libcst codegen work with ruff's Stylist | |||
| pub(crate) trait CodegenStylist<'a>: Codegen<'a> { | |||
| fn codegen_stylist(&self, stylist: &'a Stylist<'a>) -> String; | |||
There was a problem hiding this comment.
This seems to be sufficient
Suggested change
| fn codegen_stylist(&self, stylist: &'a Stylist<'a>) -> String; | |
| fn codegen_stylist(&self, stylist: &'a Stylist) -> String; |
Replace all duplicate invocations of
```rust
let mut state = CodegenState {
default_newline: &stylist.line_ending(),
default_indent: stylist.indentation(),
..CodegenState::default()
}
tree.codegen(&mut state);
state.to_string()
```
with
```rust
tree.codegen_stylist(&stylist);
```
No functional changes.
konstin
added a commit
that referenced
this pull request
Jun 13, 2023
* Abstract codegen with stylist into a CodegenStylist trait
Replace all duplicate invocations of
```rust
let mut state = CodegenState {
default_newline: &stylist.line_ending(),
default_indent: stylist.indentation(),
..CodegenState::default()
}
tree.codegen(&mut state);
state.to_string()
```
with
```rust
tree.codegen_stylist(&stylist);
```
No functional changes.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Replace all duplicate invocations of
with
No functional changes.