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

big trailing whitespace cleanup #6

Merged
1 commit merged into from
Mar 1, 2011
Merged

Conversation

Dieterbe
Copy link
Contributor

this change cleans up all trailing whitespace.

you can automatically keep it clean with something like this in your .vimrc:
autocmd BufWritePre *.py :%s/\s+$//e

the ^M's, I'm not sure where those come from, maybe when you edit the files on a Windows machine? I've never seen any unix editor add them.

* remove ^M stale characters
* strip trailing whitespace

this commit is generated with:
find . -type f | grep -vF ./.git/ | grep -v \.png$ | xargs sed -i 's/\s\+$//
@piskvorky
Copy link
Owner

Trying to apply the patch gives me:

$ curl https://github.com/piskvorky/gensim/pull/6.patch | git am
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  587k  100  587k    0     0  35469      0  0:00:16  0:00:16 --:--:-- 75332
Applying: Cleanup whitespace
error: patch failed: MANIFEST.in:1
error: MANIFEST.in: patch does not apply
error: patch failed: src/gensim/matutils.py:1
error: src/gensim/matutils.py: patch does not apply
error: patch failed: src/gensim/models/lsimodel.py:1
error: src/gensim/models/lsimodel.py: patch does not apply
error: patch failed: src/gensim/models/rpmodel.py:1
error: src/gensim/models/rpmodel.py: patch does not apply
error: patch failed: src/gensim/models/tfidfmodel.py:1
error: src/gensim/models/tfidfmodel.py: patch does not apply
Patch failed at 0001 Cleanup whitespace
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".

Any idea how to solve it?

@Dieterbe
Copy link
Contributor Author

Dieterbe commented Mar 1, 2011

hmm I believe I made that patch on top of your latest (what you pushed to github) develop branch (3a81762) so it should apply to that.

try this (from the top of my head):
git checkout develop
git remote add -f dieter git://github.com/Dieterbe/gensim.git
git rebase dieter/develop (or git merge dieter-develop if you want a merge commit)

I'm not at work so I cannot doublecheck right now, but that should be it

@piskvorky
Copy link
Owner

Had to resolve some git merge conflicts, but should be ok now.

@piskvorky
Copy link
Owner

I just noticed this removed whitespace even from blank doc comments and blank lines that separate blocks of code; this isn't right, whitespace should be justified at the level of the surrounding code.

I'll leave it like this, it's no problem for the Python parser, but I'll need to improve the find | sed combo for next time :)

Thx for bringing up the white space cleanup Dieter, these little things matter too.

@Dieterbe
Copy link
Contributor Author

Dieterbe commented Mar 5, 2011

whitespace should be justified at the level of the surrounding code.

interesting, I thought empty lines should never whitespace, even when there's code above or below, indented any level.

is this in a PEP or something?

@piskvorky
Copy link
Owner

I don't think so, at least I never saw such PEP :)

It just seems more logical/consistent (plus you can copy&paste such "properly" indented code blocks into python shell, unlike blocks which contain empty lines).

Though I expect most Python code uses the more simple greedy stripping anyway (if any), because it's easier to enforce automatically plus it's the norm in other languages.

@Dieterbe
Copy link
Contributor Author

Dieterbe commented Mar 5, 2011

where "simple greedy" is what I did, right?

This pull request was closed.
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.

2 participants