Remove blank lines instead of emptying it's contents #667
Conversation
| @@ -51,6 +51,10 @@ pub struct Match { | |||
| #[get_mut] | |||
| #[serde(skip)] | |||
| pub(crate) associated_comments: Vec<Range>, | |||
| #[get] | |||
| #[get_mut] | |||
| #[serde(skip)] | |||
There was a problem hiding this comment.
this is not just empty lines right? All leading white spaes?
There was a problem hiding this comment.
It's only empty lines, we match leading white spaces that starts with a new line character.
https://github.com/uber/piranha/pull/667/files#diff-b157ce35dcfb24fa5ff9bfb7323302e655455ad2a78bb484fdd2e855d7e740f4R143
Bennet-Sunder
force-pushed
the
remove_blank_lines
branch
2 times, most recently
from
0af76d8 to
a6c5f6b
Compare
Bennet-Sunder
force-pushed
the
remove_blank_lines
branch
from
a6c5f6b to
053f74b
Compare
|
@dvmarcilio Could you pls have a look at this PR when you have bandwidth. |
|
@Bennet-Sunder Thanks a lot for the contribution. We are still in the process of testing this internally to make sure it doesn't break any existing use-cases, but will try to merge as soon as possible. |
| end_point: Point { row: 1, column: 8 } | ||
| } | ||
| ); | ||
| } |
There was a problem hiding this comment.
Hi @Bennet-Sunder ,
Thank you for your contribution! I tested this internally and noticed that the change might break some rewrites. The issue is that in some grammars (particularly Swift), whitespace and newlines are part of the node. If you delete them, you might accidentally delete parts of a sibling node unrelated to the node you're deleting.
Please accept this test suggestion to see what I mean. The test should fail with your change (make sure to add Swift at the top). My proposed solution is to delete only whitespace or newlines until you find the end_byte of the previous sibling node. Let me know if you need help with this.
| } | |
| } | |
| #[test] | |
| fn test_switch_entry_blank_lines() { | |
| let inline_cleanup_rule = piranha_rule! { | |
| name= "inline_cleanup_rule", | |
| query= " | |
| ( | |
| (switch_entry | |
| (switch_pattern) @p1 | |
| (switch_pattern) @p2 | |
| (switch_pattern) @p3 | |
| (switch_pattern) @p4 | |
| (switch_pattern) @p5 | |
| (switch_pattern) @p6 | |
| ) @custom_entry | |
| (#eq? @p1 \".case_zeta_first\") | |
| ) | |
| ", | |
| replace_node = "custom_entry", | |
| replace = "" | |
| }; | |
| let inline_rule = InstantiatedRule::new(&inline_cleanup_rule, &HashMap::new()); | |
| let source_code = r#" | |
| public var namespace: ParameterNamespace { | |
| switch self { | |
| case .case_random_word_alpha, | |
| .case_random_word_beta, | |
| .case_random_word_gamma, | |
| .case_random_word_delta: | |
| return .namespace_group_alpha | |
| case .case_zeta_first, | |
| .case_zeta_second, | |
| .case_zeta_third, | |
| .case_zeta_fourth, | |
| .case_zeta_fifth, | |
| .case_zeta_sixth: | |
| return .namespace_group_beta | |
| case .case_random_word_omega: | |
| return .namespace_group_gamma | |
| } | |
| } | |
| "# | |
| .trim(); | |
| let piranha_arguments = PiranhaArgumentsBuilder::default() | |
| .paths_to_codebase(vec![UNUSED_CODE_PATH.to_string()]) | |
| .language(PiranhaLanguage::from(SWIFT)) | |
| .build(); | |
| let mut rule_store = RuleStore::new(&piranha_arguments); | |
| let mut parser = piranha_arguments.language().parser(); | |
| let mut source_code_unit = SourceCodeUnit::new( | |
| &mut parser, | |
| source_code.to_string(), | |
| &HashMap::new(), | |
| PathBuf::new().as_path(), | |
| &piranha_arguments, | |
| ); | |
| source_code_unit.apply_rule(inline_rule, &mut rule_store, &mut parser, &None); | |
| let transformed_code = source_code_unit.code(); | |
| let expected_code = r#" | |
| public var namespace: ParameterNamespace { | |
| switch self { | |
| case .case_random_word_alpha, | |
| .case_random_word_beta, | |
| .case_random_word_gamma, | |
| .case_random_word_delta: | |
| return .namespace_group_alpha | |
| case .case_random_word_omega: | |
| return .namespace_group_gamma | |
| } | |
| } | |
| "# | |
| .trim(); | |
| assert_eq!(transformed_code, expected_code); | |
| } | |
| @@ -16,7 +16,7 @@ use tree_sitter::Parser; | |||
| use crate::{ | |||
| filter, | |||
| models::{ | |||
| default_configs::{JAVA, UNUSED_CODE_PATH}, | |||
| default_configs::{JAVA, RUBY, UNUSED_CODE_PATH}, | |||
There was a problem hiding this comment.
| default_configs::{JAVA, RUBY, UNUSED_CODE_PATH}, | |
| default_configs::{JAVA, RUBY, SWIFT, UNUSED_CODE_PATH}, |
|
@stefanheule @danieltrt Thanks for your review of the PR. I can see that a new PR is raised with the fixes. I'll close this PR in favour of the newer one. |
Fixes the PR #667 for feature request #664 --------- Co-authored-by: Bennet Sunder <[email protected]>
Fixes #664