strip trailing whitespace from structure dump file #14328

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants

Hey,

this is really annoying to me :)
almost everybody strips trailing whitespace in their editor automatically.
it is tedious the review changes in the structure.sql when they contain changes due to trailing whitespace

what do you think?

ben

Owner

rafaelfranca commented Mar 8, 2014

instead of striping we should not being generating they. What is generating trailing whitespaces?

true that, pg_dump generates them.
i thought about piping the output in sed but wasn't sure how this works across multiple OS's

Owner

rafaelfranca commented Mar 8, 2014

maybe pg_gump should be fixed first. I don't think is Rails responsibility to fix incorrect behavior of this tool, even more in a cosmetic matter like this.

i totally agree that this is pg_dumps fault.
trailing whitespace is not just a cosmetic thing :) if you look at it in the first place you are right, but if you try working with the structure.sql file you realise that this is a real productivity killer. instead of reviewing just the change that were made to the structure.sql you have to deal with all these trailing whitespace changes. they make me 💣

if this gets fixed in a newer version of pg_dump we still have the problem that older versions of pg_dump generate an output with trailing whitespace. 💣

even though it's not rails fault i vote that rails should fix this to make the people who work with the structure.sql happier :) (especially if there are only little changes to be made)

in case you didn't notice, trailing whitespace makes me 💣 :)

Owner

rafaelfranca commented Mar 8, 2014

Yes, trailing whitespace make me 💣 too (if you look the Rails history you will see a lot of commits with ✂️ from me), but more code to me maintain make me 💥, so this need to have a good reason to me included.

I'm not a user of structure.sql so I can't know what are the downside of not accepting this, but I think almost all problems you have with trailing spaces when reviewing code can be solved with the option in the git diff that ignore whitespaces changes. Even github has support to this (append ?w=1 in the end of the diff and you will see the whitespaces changes are not showed in the diff anymore).

Anyway, I'll leave this open to someone who care more about this than I. Thank so much for the pull request and keep ✂️ the trailing whitespaces 😄

Member

senny commented Mar 8, 2014

I'm with @rafaelfranca on this one. While there is really no reason to have trailing whitespace, this is not a Rails issue. We use structure.sql with PG in our projects and it's easiest to just commit it as is.

Thank you for your work 💛

@senny senny closed this Mar 8, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment