Skip to content

Conversation

@garborg
Copy link
Contributor

@garborg garborg commented Jul 1, 2015

No description provided.

garborg added a commit that referenced this pull request Jul 1, 2015
Remove syntax deprecation warnings on 0.4
@garborg garborg merged commit a7d2368 into master Jul 1, 2015
@kmsquire
Copy link
Member

kmsquire commented Jul 1, 2015

I think I agreed with the "no space" concept until now--I think the old version was easier to read here. :-/
Cc: @johnmyleswhite

@johnmyleswhite
Copy link

There's definitely a lot to be said for exploiting vertical alignments, but I personally find it really hard to sustain smart vertical alignment like we had here when many people work on the same file. It's also one of the few things that I think is basically impossible to teach an autoformatting program to do well since it requires some semantic understanding of the code -- although the nice thing about the old code here was clearly that the semantics of the code were made more explicit by vertical alignment.

So I'm kind of torn on this. In the long run, I still really hope we'll end up with an autoformatter that standardizes style everywhere in Julia. Since I'm doubtful we could make the autoformatter be as clever as the code here used to be, I'm onboard with getting rid of semantically-inspired vertical alignment.

The only thing I would suggest here is that, once you're getting rid of this kind of vertical alignment, maybe go all the way and give up on aligning the equals signs as well? But maybe that would make the file that much worse relative to what @kmsquire liked?

@garborg
Copy link
Contributor Author

garborg commented Jul 2, 2015

Going all the way (giving up on the alignment of the equals sign) seems right to me, because I'm in on standardized style, the autoformatter isn't going to get this good (as John said), and vertical alignment inherently clutters diffs, git blames, etc., making them less useful.

I was just being timid about refactoring style, but as the lines were already changing, maybe I should have done it.

@garborg garborg deleted the depr branch July 2, 2015 02:21
@kmsquire
Copy link
Member

kmsquire commented Jul 2, 2015

Ignoring autoformatting for now (since we don't yet have an autoformatter), and going counter to what @garborg just proposed, I think I would (might) go back and add spaces after the first paren, to make things line up again.

Any problems with with that (at least until we get an autoformatter)?

@johnmyleswhite
Copy link

That's fine by me.

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.

4 participants