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

Preserving existing blank line #10

Open
flowerbug opened this issue Mar 27, 2024 · 5 comments
Open

Preserving existing blank line #10

flowerbug opened this issue Mar 27, 2024 · 5 comments

Comments

@flowerbug
Copy link

flowerbug commented Mar 27, 2024

I have beancount files with a blank line at the end, but autobean-format removes them. I don't mind if autobean-format reduces multiple blank lines to a single blank line, but removing just that one blank line at the end throws things off.

This means when I'm doing round trip comparisons it doesn't work.

It is a minor issue, but one that affects my workflow.

@SEIAROTg
Copy link
Owner

To clarify, do you have one trailing \n or two?

Squashing multiple trailing \n into one is an intentional decision as autobean-format aims to normalize all spacing in the file. This behavior is consistent with other formatters, e.g.

$ echo -n 'foo\n\n' | black - | hexdump -c
reformatted -

All done! ✨ 🍰 ✨
1 file reformatted.
0000000   f   o   o  \n
0000004

autobean-format should ensure there is exactly one trailing \n in the formatted file, unless it's an empty file (testcase). If that doesn't happen can you share steps to reproduce so I can look into this?

@flowerbug
Copy link
Author

Hello, thank you for your reply and questions.

Here is a transcript which shows what is happening.

Note, I do not mind the squashing of multiple blank lines into a single blank line. The only issue I am having is the removal of the trailing blank line (at the end).

text.txt

@SEIAROTg
Copy link
Owner

SEIAROTg commented Apr 3, 2024

Looking at your attachment, you are happy about multiple trailing \n being squashed into two, but not one, the latter being the current behavior of autobean-format.

I think it's the general convention to squashing them into one among popular formatters (e.g. black, rustfmt, clang-format) so I don't really see why autobean-format should be special here. bean-format is an outlier here as it doesn't touch line breaks at all.

This means when I'm doing round trip comparisons it doesn't work.

If that's the main issue can you do git diff --ignore-blank-lines or possibly commit the removal of extra blank lines before running autobean-format?

@flowerbug
Copy link
Author

flowerbug commented Apr 4, 2024

There is no blank line left at the end. The one \n you are seeing is at the end of the previous line. So effectively it is removing the last line of the file.

I think it would be ok if two or more \n's are squashed down to two. For some reason you are only squashing two down to one on only the last line.

I consider it inconsistent to remove that last line. :)

Perhaps we're talking past each other, but to me if you are aiming to preserve white space in a file between transactions but remove it at the end then if you are concatenating multiple files together they will no longer have the same spacing. You will end up with some transactions being put right together.

@SEIAROTg
Copy link
Owner

SEIAROTg commented Apr 4, 2024

There is no blank line left at the end. The one \n you are seeing is at the end of the previous line. So effectively it is removing the last line of the file.

I think it would be ok if two or more \n's are squashed down to two. For some reason you are only squashing two down to one on only the last line.

Yes.

I consider it inconsistent to remove that last line. :)

While \n can be seen as end of line, I think blank lines (at least visually) serves as separators rather than end of blocks. It is therefore an intentional decision to consistently remove blank lines at the beginning or end, while preserving them in the middle. Again, this aligns with the general convention, e.g.:

$ echo -n '\n\n\n\nfoo\n\n\n\nbar\n\n\n\n' | black - | hexdump -c
reformatted -

All done! ✨ 🍰 ✨
1 file reformatted.
0000000   f   o   o  \n  \n  \n   b   a   r  \n
000000a

Perhaps we're talking past each other, but to me if you are aiming to preserve white space in a file between transactions but remove it at the end then if you are concatenating multiple files together they will no longer have the same spacing. You will end up with some transactions being put right together.

Oh, are you referring to --output_mode=stdout + --recursive? Yeah, the concatenation behavior wasn't well defined there. Maybe we should always add a blank line between files, but that's not prefect either. I'd recommend --output_mode=diff or --output_mode=inplace when used with --recursive.

In principle, I think it's the responsibility of whoever does concatenation to fix the separation, while the formatter should focus on per-file formatting rather than speculating on the best separator to use between multiple files (maybe people don't want blank lines or want multiple?). I'm not aware of any obvious convention that trailing blank line should be viewed as "separator to another file" and more likely they are just redundant. However, if that is your convention, maybe you can just cat everything together than feed into autobean-format?

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

No branches or pull requests

2 participants