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

PSUseConsistentIndentation: Identation missing if command was broken over multiple lines #1406

Closed
felixfbecker opened this issue Feb 4, 2020 · 3 comments · Fixed by #1423

Comments

@felixfbecker
Copy link

felixfbecker commented Feb 4, 2020

Before submitting a bug report:

  • Make sure you are able to repro it on the latest released version
  • Perform a quick search for existing issues to check if this bug has already been reported

Steps to reproduce

Format this file: https://github.com/pcgeek86/PSGitHub/blob/5fa152568f3218843bbec97c5604d6b16b03066f/Functions/Public/New-GitHubPullRequest.ps1#L183

with

        PSUseConsistentIndentation = @{
            Enable = $true
            Kind = 'space'
            IndentationSize = 4
            PipelineIndentation = 'IncreaseIndentationForFirstPipeline'
        }

Expected behavior

Should stay the same

Actual behavior

Output (indendation wrong after the pipe):

        if ($Labels -or $TeamAssignees -or $Assignees -or $MilestoneTitle -or $MilestoneNumber) {
            # Update PR with issue properties
            $pr = $pr | Update-GitHubIssue `
                -TeamAssignees $TeamAssignees `
                -Assignees $Assignees `
                -MilestoneNumber $MilestoneNumber `
                -MilestoneTitle $MilestoneTitle `
                -Labels $Labels `
                -Token $Token |
            # Update PR object
            Get-GitHubPullRequest -Token $Token
    }

    if ($Reviewers -or $TeamReviewers) {
        $pr = $pr | New-GitHubReviewRequest -Reviewers $Reviewers -TeamReviewers $TeamReviewers -Token $Token
    }
    if ($ProjectColumnId) {
        New-GitHubProjectCard -ColumnId $ProjectColumnId -ContentType PullRequest -ContentId $pr.Id -Token $Token
    }

    $pr
}
}

If an unexpected error was thrown then please report the full error details using e.g. $error[0] | Select-Object *

Environment data

Name                           Value
----                           -----
PSVersion                      6.2.4
PSEdition                      Core
GitCommitId                    6.2.4
OS                             Darwin 19.2.0 Darwin Kernel Version 19.2.0: Sat Nov  9 03:47:04 PST 2019; root:xnu-6153…
Platform                       Unix
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

1.18.3
@ghost ghost added the Needs: Triage 🔍 label Feb 4, 2020
@bergmeister
Copy link
Collaborator

bergmeister commented Feb 9, 2020

Thanks for raising it and the level of detail. Very interesting and important. Next week is busy for me, but it's definitely on my tracker to investigate more. In the meantime I verified that the behaviour repros in master as well.
I think this could be due to code of PipelineIndentation, it only happens for the non-default options of it (default being NoIndentation).

@bergmeister
Copy link
Collaborator

A more simplified repro is this:

foo | bar |
    baz

The problem goes away if I the first line was just foo | and it does not matter if there is a comment line or if a line gets simplified to one line by not using backticks. This should be an easier starting point for being able to investigate what the logic does.

@bergmeister
Copy link
Collaborator

bergmeister commented Feb 17, 2020

First investigations show that due to having 2 pipelines in the first line, the following expression prevents an increase of the expected pipeline indentation for the rule setting IncreaseIndentationForFirstPipeline

bool isFirstPipeInPipeline = pipelineAsts.Any(pipelineAst => PositionIsEqual(((PipelineAst)pipelineAst).PipelineElements[0].Extent.EndScriptPosition, tokens[tokenIndex - 1].Extent.EndScriptPosition));

bool isFirstPipeInPipeline = pipelineAsts.Any(pipelineAst => PositionIsEqual(((PipelineAst)pipelineAst).PipelineElements[0].Extent.EndScriptPosition, tokens[tokenIndex - 1].Extent.EndScriptPosition));

This condition was added in PR #1318 as a fix for a different but similar multi-line bug. I think if we can improve this conditional logic then this bug should be resolvable without side effects. The improvement would be to not only check if the pipe is the first pipe in a Pipeline we need to check if but in addition also if there are multiple pipes on the same line that the pipe in question is the last of those.

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