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

Fix indents and trailing whitespace #21792

Closed
jdemeyer opened this issue Nov 1, 2016 · 17 comments
Closed

Fix indents and trailing whitespace #21792

jdemeyer opened this issue Nov 1, 2016 · 17 comments

Comments

@jdemeyer
Copy link

jdemeyer commented Nov 1, 2016

In all scripts: Use spaces, not a mixture of spaces and TABs to indent. Also remove all trailing whitespace.

CC: @embray @dimpase @mkoeppe

Component: scripts

Author: Jeroen Demeyer

Branch/Commit: b7d7a71

Reviewer: Matthias Koeppe, John Palmieri

Issue created by migration from https://trac.sagemath.org/ticket/21792

@jdemeyer jdemeyer added this to the sage-7.5 milestone Nov 1, 2016
@jdemeyer jdemeyer changed the title Fix indents in sage-spkg Fix indents in scripts Nov 1, 2016
@jdemeyer jdemeyer changed the title Fix indents in scripts Use spaces to indent in scripts Nov 1, 2016
@jdemeyer

This comment has been minimized.

@jdemeyer jdemeyer changed the title Use spaces to indent in scripts Fix indents and trailing whitespace Nov 1, 2016
@jdemeyer
Copy link
Author

jdemeyer commented Nov 1, 2016

Branch: u/jdemeyer/fix_indents_in_sage_spkg

@jdemeyer
Copy link
Author

jdemeyer commented Nov 1, 2016

Commit: 3e9aee3

@jdemeyer
Copy link
Author

jdemeyer commented Nov 1, 2016

comment:5

This is a totally trivial ticket. To avoid merge conflicts, a quick review would come in handy.


New commits:

3e9aee3Fix indents and trailing spaces in scripts

@dimpase
Copy link
Member

dimpase commented Nov 1, 2016

comment:7

Have you used a script to do this? If so, can we have a look at it?

@jdemeyer
Copy link
Author

jdemeyer commented Nov 1, 2016

comment:8

It was a very trivial sed script, replacing every TAB by 8 spaces and deleting all trailing spaces. I had to undo a few cases where the TABs were in Makefiles or where the TABs were not leading space.

@mkoeppe
Copy link
Member

mkoeppe commented Nov 1, 2016

comment:9

Perhaps the whitespace policy should be documented in our developer manual.

@mkoeppe
Copy link
Member

mkoeppe commented Nov 1, 2016

comment:10

And we should have a .dir-locals.el and its equivalent (if any) for vim users.
https://www.gnu.org/software/emacs/manual/html_node/emacs/Directory-Variables.html

@mkoeppe
Copy link
Member

mkoeppe commented Nov 1, 2016

comment:11

I've created ticket #21793 for that.

@mkoeppe
Copy link
Member

mkoeppe commented Nov 1, 2016

Reviewer: Matthias Koeppe

@jhpalmieri
Copy link
Member

comment:13

On my OS X machine, the change to ecm/spkg-install leads to

./spkg-install: line 293: syntax error: unexpected end of file

Undo the change and it works fine. (The - in cat >foo.c <<-"EOF" means to strip leading tabs, and << is looking for just "EOF" all by itself, so the line EOF needs to be indented strictly by tabs or not at all.)

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 1, 2016

Changed commit from 3e9aee3 to b7d7a71

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 1, 2016

Branch pushed to git repo; I updated commit sha1. New commits:

b7d7a71Don't use <<- here documents

@jdemeyer
Copy link
Author

jdemeyer commented Nov 1, 2016

comment:16

I think it's confusing to require TABs, so I used an ordinary here document with <<.

@jhpalmieri
Copy link
Member

Changed reviewer from Matthias Koeppe to Matthias Koeppe, John Palmieri

@jhpalmieri
Copy link
Member

comment:17

Okay, looks good.

@vbraun
Copy link
Member

vbraun commented Nov 7, 2016

Changed branch from u/jdemeyer/fix_indents_in_sage_spkg to b7d7a71

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

No branches or pull requests

5 participants