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

Leaf content trimming is causing unforeseen behaviours #492

Closed
nbacquey opened this issue May 31, 2023 · 4 comments · Fixed by #493
Closed

Leaf content trimming is causing unforeseen behaviours #492

nbacquey opened this issue May 31, 2023 · 4 comments · Fixed by #493
Labels
language: bash Bash formatting issues type: bug

Comments

@nbacquey
Copy link
Member

In pretty.rs, we have the following piece of code:

            Atom::Leaf {
                content,
                single_line_no_indent,
                ..
            } => {
                // [...]
                write!(buffer, "{}", content.trim_end())?;
            }

The trim_end() part is the cause of #488: it deletes whitespaces at the end of leaves, even when they are part of the semantics.

The way I understand it, trim_end() has been added here to avoid printing leaf nodes containing a single "\n" in Bash. I think it would be better to explicitly @delete those nodes in the query, instead of silently ignoring them in the pretty-printing phase.
Line breaks are part of the semantics in Bash, I think they ought to be processed with the query mechanism, and the trim_end() call should be removed.

@Xophmeister, do you have additional insight on this issue?

@Xophmeister
Copy link
Member

AFAIK, the trim_end has always been there; it wasn't added to resolve the \n nodes in Bash, which are still an issue. Does a git blame show the provenance?

@torhovland
Copy link
Member

It may have been added as a cheap way to get rid of white-space at end of lines. I suggest you try to remove it, and then possibly add a post-processing step or fix some queries to get green tests again.

@Xophmeister
Copy link
Member

(The post-processing step to remove whitespace at the end of lines already exists. As such, you may be able to just remove this trim_end with no consequence.)

@nbacquey
Copy link
Member Author

I tried to remove it, and it broke the I/O tests for Bash, hence my assumption.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
language: bash Bash formatting issues type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants