Jump to content

Code Style (Revisited)


Akinari

Recommended Posts


  • Group:  Members
  • Topic Count:  32
  • Topics Per Day:  0.01
  • Content Count:  247
  • Reputation:   207
  • Joined:  10/23/12
  • Last Seen:  

I was following the other topic about code style here and it ended up simply making the files bigger and source harder to deal with while making more work for people with modifications as stated in the first post in that topic. I can only guess it might have been unintentional, but like Brian, I would have to agree that 4 spaces = tab is the correct way to go. Is there a final verdict on whether the files are staying like this or will it once again be modified?

  • Upvote 4
Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  8
  • Topics Per Day:  0.00
  • Content Count:  148
  • Reputation:   46
  • Joined:  11/02/11
  • Last Seen:  

Do you mean the spaces instead of tabs? You should ask GreenBox.

Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  75
  • Topics Per Day:  0.02
  • Content Count:  2223
  • Reputation:   593
  • Joined:  10/26/11
  • Last Seen:  

*moves my post here*

@GreenBox: was the conversion of indents from tabs to spaces intended?

I want to cry every time I see you guys using spaces everywhere to indent while like 98% of source use tab :P

I also vote for tab indents /rice

# Indent using tabs. Convert 4 spaces to 1 tab.
--indent=tab=4

  • Upvote 4
Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  19
  • Topics Per Day:  0.00
  • Content Count:  713
  • Reputation:   70
  • Joined:  11/08/11
  • Last Seen:  

oh God, bring the tabs back please! xd

Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  9
  • Topics Per Day:  0.00
  • Content Count:  303
  • Reputation:   101
  • Joined:  11/13/11
  • Last Seen:  

We discussed tabs vs spaces over IRC and as far I remember we choosed spaces.

Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  8
  • Topics Per Day:  0.00
  • Content Count:  148
  • Reputation:   46
  • Joined:  11/02/11
  • Last Seen:  

Refresh my mind as to the reasons why spaces over tabs.

Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  75
  • Topics Per Day:  0.02
  • Content Count:  2223
  • Reputation:   593
  • Joined:  10/26/11
  • Last Seen:  

/swt Spaces advantages:

+ all indentation is guaranteed to look the same in all text editors

/no1 TABs advantages:

+ smaller file size (each tab is 1 character vs 4 spaces is 4 characters)

+ users set their own TAB display size in their text editors

+ editors that color tabs differently than spaces (might make TABs stand out more than 4 space indent?)

  • Upvote 2
Link to comment
Share on other sites


  • Group:  Development Manager
  • Topic Count:  56
  • Topics Per Day:  0.01
  • Content Count:  732
  • Reputation:   525
  • Joined:  12/13/11
  • Last Seen:  

I was about to create a new topic as well since we couldn't reply in the one marked as implemented. I would vote for tabs back as well. By default many editors use tabs automatically for indentation, Netbeans being one of the widely used ones that uses spaces by default that I've ever really used.

It also breaks all of the older diffs that were created long ago that use tabs instead of spaces.

Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  42
  • Topics Per Day:  0.01
  • Content Count:  227
  • Reputation:   11
  • Joined:  11/16/11
  • Last Seen:  

It broke 6 of my src modification that uses tabs and had to re-do it to use spaces which took me a lot of time wasted hitting space bar just to make it more readable. Good thing my space bar didn't break lol.

Edited by RaGERO
Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  20
  • Topics Per Day:  0.00
  • Content Count:  213
  • Reputation:   109
  • Joined:  05/21/12
  • Last Seen:  

We discussed tabs vs spaces over IRC and as far I remember we choosed spaces.

I don't remember us specifically stating this, to be honest. We might want to pull IRC logs--although, it's not necessary, imo. Next time, I'll post a quote-box with our IRC chat though.

Refresh my mind as to the reasons why spaces over tabs.

In all honesty, we'll need to make a decision on this and quickly.

As I said, every time, I'm indifferent to this thread. I don't have a specific preference, to be honest. I do have to agree with Brian's post for logical reasons.

Edit:

I've opened a PM with @GreenBox. We will update this thread.

Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  9
  • Topics Per Day:  0.00
  • Content Count:  554
  • Reputation:   70
  • Joined:  04/04/12
  • Last Seen:  

I also vote for tab indents

+1

hmm..that is why my patches are broke..:(

hehe..need to recode everything before commiting....

:meow:

Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  4
  • Topics Per Day:  0.00
  • Content Count:  67
  • Reputation:   75
  • Joined:  12/18/11
  • Last Seen:  

An unsafe action to generally take is to convert spaces to tabs or vice-versa in the entire repository. The biggest problem this causes is that history is lost from before and after this change.

I would highly advise against changing the formatting of large blocks of code in the current repository so as not to break diffs and lose history for those sections of code.

For example, if I went and changed every indendation of every file in the repository from spaces/tabs to non-breaking spaces (hypothetically), any diffs that I had would no longer work with the updated repository files.

I don't know about SVN, but when we are using git, a developer can create filters that are executed when 1. adding files to the repository or 2. checking out files from the repository at which point one can convert from spaces to tabs or vice-versa (http://git-scm.com/b...yword-Expansion).

I would recommend that a standard be decided, and that new code must either follow the standard or follow the style of the file that the code is being added to, if it differs consistently from the proposed standard.

  • Upvote 2
Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  13
  • Topics Per Day:  0.00
  • Content Count:  99
  • Reputation:   10
  • Joined:  11/21/11
  • Last Seen:  

It also breaks all of the older diffs that were created long ago that use tabs instead of spaces.

Please put back tab!!!

Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  6
  • Topics Per Day:  0.00
  • Content Count:  134
  • Reputation:   35
  • Joined:  02/27/12
  • Last Seen:  

I vote for tabs.

  • Upvote 1
Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  21
  • Topics Per Day:  0.00
  • Content Count:  326
  • Reputation:   19
  • Joined:  09/27/12
  • Last Seen:  

Yes, I am in agreement with Brian and others voting for tabs. At this time, changing it to spaces inconsistently breaks many source modifications that people have already put in... even minor ones.

For example, I put a single line source modification, with 1 number changed (200 instead of 2000)... and it broke in this update. The entire file was conflicted because Tortoise did not recognize the line anymore.

It also makes any files that I have source modded in the past, even just adding new blocks that don't even affect the source, also cause conflicts.

For now I'm not going to update until I know for sure that this is what we are going with; but me personally... please bring the tabs back.

Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  4
  • Topics Per Day:  0.00
  • Content Count:  67
  • Reputation:   75
  • Joined:  12/18/11
  • Last Seen:  

It will break diffs either way if we switch to spaces or tabs -- that is, if we change existing code just because we think it "looks better" -- the style discussed here really must only apply to new code, not blindly applied to existing code.

Can we in the meantime revert Greenbox's commit where he changed the leading whitespace of thousands of lines ?

  • Upvote 3
Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  20
  • Topics Per Day:  0.00
  • Content Count:  213
  • Reputation:   109
  • Joined:  05/21/12
  • Last Seen:  

Agreed with Trojal's post. Until this is properly decided upon, and in the meantime, we'll revert.

Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  75
  • Topics Per Day:  0.02
  • Content Count:  2223
  • Reputation:   593
  • Joined:  10/26/11
  • Last Seen:  

The biggest problem this causes is that history is lost from before and after this change.

Yes, now SVN Blame will blame the person who committed the TAB-to-spaces conversion. And when we revert this, SVN Blame will blame that person too! We'll have to do two "Blame previous revision" lookups to find out who actually added/edited a line!

The only way to prevent that is:

  1. undo r16968 by doing SVN Replace on the whole /trunk/src/ (replace it with revision 16967 of /trunk/src)
  2. have everyone re-commit r16969 to r16991 (13 commits to /trunk/src)
  3. then do the proper AStyle conversion (4 spaces = 1 tab)
    - this will still cause converted lines to blame the wrong person
    (and when looking up SVN Blame you have to do "Blame previous revision")
    + but at least you only have to do Blame previous revision ONCE
    + and it will only be for the lines that were space-indented, then converted to tabs
    (I'm guessing less than 5% of the code? The majority of it was already indented with TABs prior to r16968.)

  • Upvote 1
Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  16
  • Topics Per Day:  0.00
  • Content Count:  737
  • Reputation:   216
  • Joined:  11/29/11
  • Last Seen:  

Pretty useless to revert back imo.

Yes it do break lot of diff but those was going to be break one day anyways.

Updating diff is just a revert patch, KR, update, conflict operation, so yeah it not pleasant but it's not that "omg" and to repeat myself the diff is not for infinite duration so at least we know is up to date.

Now let say we revert this and apply KR for new code, what does that mean, that only new file will have that code style ? That only when we edit a module we apply KR ? only for a block ?

This was meant to harmonize code style, with this reverse will only get back to square one, but yeah at least we more or less agreed on wich style to configure for IDE. It will just postpond diff breaking when someone will edit the file anyways, so ok some file ain't touch a lot so that a bit less conflicts but that all...

Finally replace will break all index we put on bugtracker and in commit message; follow up:x rev:x etc all gonna be wrong.

You can always if you don't like it update to head, cherrypick -16968, tryto apply your fix.

Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  13
  • Topics Per Day:  0.00
  • Content Count:  99
  • Reputation:   10
  • Joined:  11/21/11
  • Last Seen:  

The problem is that lot of server have made a lot of src modification and now to update to this they need to restart all they work. It's to bad and it's take to much time. And as I can read the majority want to get tab so why not put tab? More we wait more the rollback will be really difficult. Please make a quick dessission see If admin restart they works or if they should wait a bit.

Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  16
  • Topics Per Day:  0.00
  • Content Count:  737
  • Reputation:   216
  • Joined:  11/29/11
  • Last Seen:  

Tab or space ain't the only thing who's breaking your diff. It just that since there a lot many block was marked as changed in svn, and when a block encounter a local change => boom conflict.

I also had lot of src modification, it's quick if you know how to do it and I doubt pserv put as many reference as we do, (track wich revision did what etc)

Just : svn co http/ra new_repo/; svn merge -c -16968; diff new_repo myrepo; apply diff

Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  13
  • Topics Per Day:  0.00
  • Content Count:  99
  • Reputation:   10
  • Joined:  11/21/11
  • Last Seen:  

I have about 1500 diffs on my server only for src. 50% of time I need to update manually but with this update I need to redo all if I want to update automaticly th 50% of the other time.

Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  6
  • Topics Per Day:  0.00
  • Content Count:  134
  • Reputation:   35
  • Joined:  02/27/12
  • Last Seen:  

I just think Cookie has to approve it soon, if it's gonna be approved, so we can send the the revision 16968 again and reput the diffs.

Since there's just 14 changeset of Core after this, which are all 5~10 min(with excption of 16981 which will take maybe 25 min) of copy/paste or less each, it doesn't seem much things to do.

Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  20
  • Topics Per Day:  0.00
  • Content Count:  213
  • Reputation:   109
  • Joined:  05/21/12
  • Last Seen:  

Any of the Core Devs feel free to take care of this as I'm out of commission this weekend as posted.

Thanks,

Cookie

Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  21
  • Topics Per Day:  0.00
  • Content Count:  326
  • Reputation:   19
  • Joined:  09/27/12
  • Last Seen:  

Pretty useless to revert back imo.

Yes it do break lot of diff but those was going to be break one day anyways.

Updating diff is just a revert patch, KR, update, conflict operation, so yeah it not pleasant but it's not that "omg" and to repeat myself the diff is not for infinite duration so at least we know is up to date.

I don't agree with this at all. The main reason for changing it to the spaces was to make the code look uniform in specific editors... but the fact of the matter is - the code looked the same in a particular editor after it was initially made for each person that looks at it. Therefore, people who use MSVS are used to the visual studio look, regardless of tabs, and those who are doing it in Linux see the same thing regardless... and so on and so forth. It only really comes into effect when you are comparing your work with someone else's on a different platform... which I've never understood was that huge of a concern.

Like, why make more work for everyone just to make the code look "prettier"?

That being said, I agree with Brian's original assessment. The spaces are more problems than they are worth in addition to breaking diffs. This includes file size and association with programs that default to tabs and the other stuff he said much better in his post. XD

Link to comment
Share on other sites

×
×
  • Create New...