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.

Sign up for GitHub

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

Jump to bottom

Features/#148 multiline function call in a single-line declared section #392

Merged
merged 4 commits into from
Jul 29, 2021
Merged

Features/#148 multiline function call in a single-line declared section #392

merged 4 commits into from
Jul 29, 2021

Conversation

dakochik
Copy link
Contributor

Issue #148 is resolved

iromeo
iromeo requested changes Jul 26, 2021
View reviewed changes
Copy link
Contributor

@iromeo iromeo left a comment

Choose a reason for hiding this comment

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

Not everything works as expected

src/main/kotlin/com/jetbrains/snakecharm/inspections/SmkMultilineFunctionCallInspection.kt Outdated Show resolved Hide resolved
src/main/kotlin/com/jetbrains/snakecharm/inspections/SmkMultilineFunctionCallInspection.kt Outdated Show resolved Hide resolved
src/main/kotlin/com/jetbrains/snakecharm/inspections/SmkMultilineFunctionCallInspection.kt Outdated Show resolved Hide resolved
src/main/kotlin/com/jetbrains/snakecharm/inspections/SmkMultilineFunctionCallInspection.kt Show resolved Hide resolved
src/main/kotlin/com/jetbrains/snakecharm/inspections/SmkMultilineFunctionCallInspection.kt Outdated Show resolved Hide resolved
src/main/kotlin/com/jetbrains/snakecharm/inspections/SmkMultilineFunctionCallInspection.kt Show resolved Hide resolved
@iromeo iromeo assigned dakochik and unassigned iromeo Jul 26, 2021
@iromeo iromeo self-requested a review July 26, 2021 16:17
fixed: quickfix was modified
e7e2d56 
refactored: inspection code, SmkRuleOrCheckpointArgsSection got new function

We create separate method in SmkRuleOrCheckpointArgsSectionImpl in order not to duplicate code, if there any other SmkRuleOrCheckpointArgsSection inheritor, which will need the same logic e.g. 'use' section. Other variant is to move multilineSectionDefinition() implementation into SmkRuleOrCheckpointArgsSection interface but I'm not sure if it is a good idea to mix definition and implementation.

Resolves: #148
@dakochik
Copy link
Contributor Author

fixed: quickfix was modified, so now it suggests to transform single-line declared section to multiline declared as follows:

# before:
rule rule_148_b:
    input:  foo1("text1",
                "text2", "text3"), foo2("text4")

# after:
rule rule_148_b:
    input: 
        foo1("text1",
            "text2", "text3"),
        foo2("text4")

refactored: multilineSectionDefinition() was moved into SmkRuleOrCheckpointArgsSection, see commit message, some minor changes in inspection code, which were requiered

@dakochik dakochik assigned iromeo and unassigned dakochik Jul 28, 2021
iromeo
iromeo requested changes Jul 28, 2021
View reviewed changes
Copy link
Contributor

@iromeo iromeo left a comment

Choose a reason for hiding this comment

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

Please get rid of harcoded TAB meaning. In general everything works good.

src/main/kotlin/com/jetbrains/snakecharm/lang/psi/impl/SmkRuleOrCheckpointArgsSectionImpl.kt Outdated Show resolved Hide resolved
src/main/kotlin/com/jetbrains/snakecharm/inspections/SmkMultilineFunctionCallInspection.kt Outdated
import com.jetbrains.snakecharm.SnakemakeBundle
import com.jetbrains.snakecharm.lang.psi.SmkRuleOrCheckpointArgsSection

const val TAB = " "
Copy link
Contributor

Choose a reason for hiding this comment

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

We cannot use such TAB def here, number of spaces in TAB is IDE setting. More over user could use real TAB character instead of spaces.

Copy link
Contributor

Choose a reason for hiding this comment

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

And is only setting for format code features, indents in user file could be different
image

src/main/kotlin/com/jetbrains/snakecharm/inspections/SmkMultilineFunctionCallInspection.kt Outdated Show resolved Hide resolved
@iromeo iromeo assigned dakochik and unassigned iromeo Jul 28, 2021
@dakochik
Copy link
Contributor Author

fixed: TAB hardcoding, 'contains' function calling

@dakochik dakochik assigned iromeo and unassigned dakochik Jul 29, 2021
@iromeo
refactor: 'psiElement.prevSibling' isn't an field access, is a comple…
1f16920 
…x method, let's cache it value in local variable
@iromeo iromeo merged commit de8adcd into master Jul 29, 2021
@iromeo iromeo deleted the features/#148-multiline-function-call-inspection branch July 29, 2021 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iromeo iromeo iromeo requested changes

Assignees

@iromeo iromeo

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dakochik @iromeo