Skip to content

Avoid trailing whitespaces #5

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

Closed
lukpueh opened this issue Oct 27, 2016 · 7 comments · Fixed by #21
Closed

Avoid trailing whitespaces #5

lukpueh opened this issue Oct 27, 2016 · 7 comments · Fixed by #21

Comments

@lukpueh
Copy link
Member

lukpueh commented Oct 27, 2016

I recently started using a text editor plugin that unveils trailing white spaces in a funky color. This makes all of our projects look like pieces of modern art.

We do say that our guidelines are based on Pep 8 which mentions that one should avoid trailing white spaces for good reasons (other than distracting people with said plugins).

Does anyone care for a tiny addition to the guidelines, encouraging using text editor plugins/options that show trailing whites paces, and or linters like flake8 which warn about them? Or should I just stop using my plugin?

@vladimir-v-diaz
Copy link
Contributor

I think it's better if we remove any artwork found in the code.

@aaaaalbert
Copy link
Contributor

I'm also all for removing trailing whitespace on encounter. However, I'm not so sure about creating a new style rule especially for this....

@awwad
Copy link
Contributor

awwad commented Oct 27, 2016

I think that’s sensible (advising use of editors that note trailing whitespace).

I don’t happen to use such a plugin, but git diff will yell at you if you include trailing whitespace, and I use that pre-commit pretty consistently so I generally see them and have been removing them in the code base - though only where I touch lines (so as not to generate massive, confusing, mostly-empty diffs).

On that note, large empty diffs are bad. We should decide to edit only when things are touched and/or kick off a big change only when a large proportion of branches are currently merged - maybe after releases?

On Oct 27, 2016, at 15:52, lukpueh notifications@github.com wrote:

I recently started using a text editor plugin that unveils trailing white spaces in a funky color. This makes all of our projects look like pieces of modern art.

We do say that our guidelines are based on Pep 8 which mentions that one should avoid trailing white spaces https://www.python.org/dev/peps/pep-0008/#other-recommendations for good reasons (other than distracting people with said plugins).

Does anyone care for a tiny addition to the guidelines, encouraging using text editor plugins/options that show trailing whites paces, and or linters like flake8 which warn about them? Or should I just stop using my plugin?


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub #5, or mute the thread https://github.com/notifications/unsubscribe-auth/AMpkOPq4llpR8qUdZs3UaN5tDrAuWbbTks5q4QDmgaJpZM4KiwTJ.

@JustinCappos
Copy link
Contributor

I think your plug-in is helpful. I don't know we need to require that
people use a plug-in, but suggesting it and reiterating the need to avoid
trailing spaces makes sense to me.

On Thu, Oct 27, 2016 at 3:52 PM, lukpueh notifications@github.com wrote:

I recently started using a text editor plugin that unveils trailing white
spaces in a funky color. This makes all of our projects look like pieces of
modern art.

We do say that our guidelines are based on Pep 8 which mentions that one
should avoid trailing white spaces
https://www.python.org/dev/peps/pep-0008/#other-recommendations for
good reasons (other than distracting people with said plugins).

Does anyone care for a tiny addition to the guidelines, encouraging using
text editor plugins/options that show trailing whites paces, and or linters
like flake8 which warn about them? Or should I just stop using my plugin?


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#5,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA0XD6FC3_ibFk2-5qhf33Qqwu7d9k6Eks5q4QDlgaJpZM4KiwTJ
.

@lukpueh
Copy link
Member Author

lukpueh commented Nov 23, 2016

Just found a neat hidden feature on GitHub secrets.
"Add ?w=1 to the URL to see the diff with whitespace ignored."

@SantiagoTorres
Copy link

Adding to the git diff suggestion, git diff can take the --check flag to tell you common errors in your commit code:

santiago at ~/tuf ✔ git diff --check HEAD^
src/tuf/tests/simple_server.py:4: trailing whitespace.
+ 
src/tuf/tests/simple_server.py:10: trailing whitespace.
+  
src/tuf/tests/simple_server.py:15: trailing whitespace.
+  This is a basic server that was designed to be used in conjunction with 
src/tuf/tests/simple_server.py:16: trailing whitespace.
+  test_download.py to test download.py module. 
src/tuf/tests/test_download.py:19: trailing whitespace.
+Otherwise, module that launches simple server would not be found.  
src/tuf/tests/test_download.py:75: trailing whitespace.
+    self.target_hash = {'md5':digest}  
src/tuf/tests/test_download.py:103: trailing whitespace.
+                      required_hashes=self.target_hash) 
src/tuf/tests/test_download.py:110: trailing whitespace.
+                      required_hashes=self.target_hash, 
src/tuf/tests/test_download.py:117: trailing whitespace.
+    self.assertRaises(tuf.DownloadError, 
src/tuf/tests/test_download.py:119: trailing whitespace.
+                      required_hashes=self.target_hash, 
src/tuf/tests/test_download.py:122: trailing whitespace.
+    self.assertRaises(tuf.DownloadError, 
src/tuf/tests/test_download.py:124: trailing whitespace.
+                      required_hashes=self.target_hash, 
src/tuf/tests/test_download.py:128: trailing whitespace.
+    self.assertRaises(tuf.DownloadError, 
src/tuf/tests/test_download.py:136: trailing whitespace.
+                      required_hashes=self.target_hash, 
src/tuf/tests/test_download.py:142: trailing whitespace.
+                      required_hashes=self.target_hash, 
src/tuf/tests/test_download.py:148: trailing whitespace.
+                      required_hashes=self.target_hash, 
src/tuf/tests/test_download.py:154: trailing whitespace.
+                      required_hashes=self.target_hash, 
src/tuf/tests/test_download.py:164: trailing whitespace.
+                      required_hashes=self.target_hash, 
src/tuf/tests/test_download.py:168: trailing whitespace.
+    end_real = time.time()  

(This looks better with colored output)

@lukpueh
Copy link
Member Author

lukpueh commented Nov 18, 2020

The newly referenced Google style guide mentions avoiding trailing whitespace, and so does PEP 8. Moreover, linters will easily flag this. I suggest we close the issue with #21.

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 a pull request may close this issue.

6 participants