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

Make the handling of inserted newlines more consistent #144

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

chadrik
Copy link

@chadrik chadrik commented Jul 19, 2017

This removes a bunch of special-case addition of newlines and attempts to preserve exactly the number provided by the user when inserting or setting. It also fixes #48, where inserting a line after a 'def' could produce additional newlines after the 'def' block.

The erroneous newline triggered by insertion (issue #48) is due to this:

        if not expected_list or not isinstance(expected_list[-1], (CodeBlockNode, redbaron.nodes.EndlNode)):
            log(
                ">> List is empty or last node is not a CodeBlockNode or EndlNode, append a separator to it and set identation to it")
            expected_list.append(generate_separator())
            expected_list[-1].indent = last_indentation
            log("-- current result: %s", ["".join(map(lambda x: x.dumps(), expected_list))])

The sensible solution seemed to be to make IfelseblockNode inherit from CodeBlockNode (it's a code block after all), which removed the stray newline, but that made the following test fail:

def test_line_proxy_correctly_indent_code_block():
    red = RedBaron("while True:\n    pass\n")
    red[0].extend(["if a:\n    pass\n\n"])
    assert red.dumps() == "while True:\n    pass\n    if a:\n        pass\n\n"

The new result lost the final newline. This led me into CodeBlockNode.parse_code_block, where I discovered a lot of questionable (to an outsider) removal and addition of newlines.

It starts with:

            clean_string = "    " + "\n    ".join(clean_string.split("\n"))
            clean_string = clean_string.rstrip()

Note that the call to strip removes whitespace and newlines.

Then there was:

        fst = baron.parse("def a():\n%s\n" % clean_string)[0]["value"]

The %s\n adds a newline.

Then there was a bunch of code trying to guess how many newlines to add back, without taking into consideration how many there were to begin with. I ditched all of that. This means that this PR changes the expected results of the setter tests, but it does so in a way that is truer to the user's request.

e.g.

    c.value = "return 42\n"

Now inserts a single newline as requested, instead of 3.

There is a very good chance that I've misunderstood something, since I've only just started using redbaron. Please let me know if that is the case.

@chadrik chadrik changed the title Simplify the handling of line-endings. Make the handling of inserted newlines more consistent Jul 19, 2017
This removes a bunch of special-case addition of newlines and attempts to preserve exactly the number provided by the user when inserting or setting.
It also fixes a bug where inserting a line after a 'def' could produce additional newlines after the 'def' block.

Note that this does change the expected results of the setter tests.
@webknjaz
Copy link
Contributor

webknjaz commented Jan 7, 2020

@Psycojoker ^

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.

whitespace changes on code insertion
2 participants