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

Some whitespace cleanup #85

Closed
wants to merge 3 commits into from
Closed

Some whitespace cleanup #85

wants to merge 3 commits into from

Conversation

Jorge-C
Copy link
Contributor

@Jorge-C Jorge-C commented Feb 7, 2014

My editor likes to make lines with trailing whitespace red, so I'd better fix that to keep sane.

By the way, the first and last lines of this range have a trailing space. If we can remove those then pep8 bipy passes. Currently we run pep8 --ignore=W which must have some kind of bug because it raises errors for stuff that isn't an error when not ignoring warnings (!).

In emacs this can be automated by adding (add-hook 'before-save-hook 'delete-trailing-whitespace) to .emacs.

This shouldn't mess with any of the files being modified/added in the other PRs.

There're two trailing spaces in bipy/core/tests/test_distance (lines 435 and
448) that aren't removed because maybe they're there on purpose.
@jairideout
Copy link
Contributor

The whitespace you noted in test_distance.py shouldn't be removed because it's testing that the parser handles lines with leading/trailing whitespace.

@jairideout
Copy link
Contributor

Though I agree it'd be good to get pep8 bipy to work without warnings disabled. Maybe I should just do without the multiline string and put it all in a single string?

@wasade
Copy link
Collaborator

wasade commented Feb 7, 2014

This works for vim too:

autocmd BufWritePre *.py :%s/\s\+$//e

@Jorge-C
Copy link
Contributor Author

Jorge-C commented Feb 7, 2014

Checking if pep8 had a way to diable whitespace checking in string literals (it doesn't) I found that PEP8 discourages it:

Don't write string literals that rely on significant trailing whitespace. Such trailing whitespace is visually indistinguishable and some editors (or more recently, reindent.py) will trim them.

Would you prefer to set var = '\n'.join(['ln1 ', 'ln2', ...]) or var = 'ln1 \nln2....'?

@jairideout
Copy link
Contributor

Sounds good. I like the former (using join)- do you mind taking care of this in this pull request?

@Jorge-C
Copy link
Contributor Author

Jorge-C commented Feb 7, 2014

Sure, will do!
On Feb 7, 2014 9:36 AM, "Jai Ram Rideout" notifications@github.com wrote:

Sounds good. I like the former (using join)- do you mind taking care of
this in this pull request?


Reply to this email directly or view it on GitHubhttps://github.com//pull/85#issuecomment-34464822
.

@jairideout
Copy link
Contributor

Thanks!

@wasade
Copy link
Collaborator

wasade commented Feb 7, 2014

Since we're on the discussion of pep8, I've really found Syntastic to be very helpful. It automatically runs pep8 on each save and easily flags the issues.

@jairideout
Copy link
Contributor

Planning on using it when I can find some spare time to try it out- looks really useful

@Jorge-C
Copy link
Contributor Author

Jorge-C commented Feb 7, 2014

Nice! Just in case there're more emacs users, I use elpy, which among other things includes pyflakes and pep8 for static checking and style issues. It works great :)

@jairideout
Copy link
Contributor

You don't happen to also use tcsh?

@ElDeveloper
Copy link
Contributor

It's a sign!!

On Feb 7, 2014, at 9:55 AM, Jai Ram Rideout notifications@github.com wrote:

You don't happen to also use tcsh?


Reply to this email directly or view it on GitHub.

@Jorge-C
Copy link
Contributor Author

Jorge-C commented Feb 7, 2014

@jrrideout Nope, the one I use is definitely not an "enhanced" anything like tcsh's website says. And that's why I end up doing a lot of terminal work from the IPython console.

@Jorge-C
Copy link
Contributor Author

Jorge-C commented Feb 7, 2014

Sorry, I made this PR from my master branch (bad idea), so closing and sending a new one.

@Jorge-C Jorge-C closed this Feb 7, 2014
@Jorge-C Jorge-C mentioned this pull request Feb 7, 2014
@jairideout
Copy link
Contributor

Darn, I thought I'd found @cleme's doppelganger

@cleme
Copy link

cleme commented Feb 7, 2014

One day you'll all leave the Dark side of the Force and join emacs. Actually, now that I run my own lab I should enforce it: you work here, you use emacs and tcsh.

Oh wait, people in my lab don't listen to me.

@ElDeveloper
Copy link
Contributor

Specially when people in your lab are rowdy whiskey addicts.

On Feb 7, 2014, at 11:27 AM, Jose Carlos Clemente Litran notifications@github.com wrote:

One day you'll all leave the Dark side of the Force and join emacs. Actually, now that I run my own lab I should enforce it: you work here, you use emacs and tcsh.

Oh wait, people in my lab don't listen to me.


Reply to this email directly or view it on GitHub.

@Jorge-C
Copy link
Contributor Author

Jorge-C commented Feb 7, 2014

Come join us, we don't have cookies but jedis are way cooler :)

@jairideout
Copy link
Contributor

🍺 🍺 🍻 🍺 🍺

Why doesn't github have a whiskey emoji???

@ElDeveloper
Copy link
Contributor

wdjixs8

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.

None yet

5 participants