Skip to content

fix: transform inputs with dedent#14822

Merged
Carreau merged 2 commits intoipython:mainfrom
Erotemic:dev/fix_dedent
Mar 7, 2025
Merged

fix: transform inputs with dedent#14822
Carreau merged 2 commits intoipython:mainfrom
Erotemic:dev/fix_dedent

Conversation

@Erotemic
Copy link
Copy Markdown
Contributor

@Erotemic Erotemic commented Mar 5, 2025

fixes: #14818

The leading_indent function in IPython/core/inputtransformer2.py had an issue in that it checked to see if the first line had a comment anywhere in it, and if it did, then it just would ignore checking if the rest of the input could possibly need to have indentation removed.

The main issue is that it looks like the function assumes you can just check the first line, but that is not the case. You need to check the indentation level of every line in the input and remove the minimum (in terms of length) indentation common to all lines.

I'm not sure why this function isn't just using textwrap.dedent, which seems like it does exactly what we want to be doing. I just switched the function to use this.

For the tests, it turns out that there was a test cases that wasn't even being checked. I fixed that as well and added the sample I noted in #14818 as a 3rd test case.

While working on this, I wrote a more involved test, which didn't really fit with the style of the other tests, but I'll put it here in case you want to include it.

Details
def test_regression_14818():
    from IPython.core import inputtransformer2 as ipt2
    from textwrap import dedent

    def indent(text, prefix='    '):
        """Indents a block of text"""
        return prefix + text.replace('\n', '\n' + prefix)

    targets = {}
    targets['two_statements'] = dedent(
        """
        x = 1
        print(x)
        """
    ).strip()
    targets['aligned_comment_and_two_statements'] = dedent(
        """
        # This is a comment
        x = 1
        print(x)
        """
    ).strip()
    targets['misaligned_comment_and_two_statements'] = dedent(
        """
            # This is a misaligned comment
        x = 1
        print(x)
        """
    ).strip()

    # Check that all of the targets are valid python
    for target_name, target_code in targets.items():
        exec(target_code, {}, {})

    # Modify the target text to test that ipt2.leading_indent can undo the
    # modifications.
    cases = []
    for target_name, target_code in targets.items():
        cases.append({
            'name': target_name + '_space',
            'sample': indent(target_code, prefix='    '),
            'target': target_code,
        })
        cases.append({
            'name': target_name + '_tab',
            'sample': indent(target_code, prefix='\t'),
            'target': target_code,
        })

    for case in cases:
        case_name = case['name']
        lines = case['sample'].splitlines(keepends=True)
        recon_lines = ipt2.leading_indent(lines)
        reconstructed = ''.join(recon_lines)
        assert reconstructed == case['target'], f'failed case {case_name}'

@Carreau Carreau added this to the 9.0.2 milestone Mar 6, 2025
@Carreau
Copy link
Copy Markdown
Member

Carreau commented Mar 7, 2025

Thanks, this is quite old code, that dates back from readlines, when you could only get lines one by one, let's try it as is, and see if there are any other issues, in which case we'll have to add more tests.

@Carreau Carreau merged commit 3c6a7a3 into ipython:main Mar 7, 2025
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Regression in 9.0.1 from 8.33.0 - No longer dedents inputs.

2 participants