Help @dg as per the deal to convert tabs to spaces.

[harikt]

Hi David Grudl,

As per our conversation and your deal I am sending you my first PR to help you with the conversion.

I will start with the rest once you have merged this.

Thank you

[harikt]

@dg I was looking at your conversation https://twitter.com/geekovo/status/507984028348461057 .

If you don't like spaces, I have no problem. I send you the PR for you asked. And I believe it was not a joke.

[Majkl578]

@harikt: Even though I disagree with @dg and consider his behavior as purely childish, this is not any better...

---

But yes, forcing spaces is bullshit and a main reason for me to ignore / not use PSR-2.
By the way, php-fig/fig-standards#35 and php-fig/fig-standards#91 also contain pretty interesting discussion, which just proves how PHP-FIG members are reluctant and ignorant to PHP user-base as whole. There was no really persuasive argument for using spaces, but still, ignorance (or arrogance?) wins over constructive discussion/argumentation. But you know, they don't care, they just made those standards for themselves (but the fact they push it hard to PHP user-land is pretty contradictory).

[nechutny]

Why spaces? Files are bigger, peoples can't set own indention width...

1940s: Various "computers" are "programmed" using direct wiring and switches. Engineers do this in order to avoid the tabs vs spaces debate.

[dg]

@harikt thank you. Let's convert whole nette/utils to spaces. Now we need to convert pull requests, at least first one (#20), to demonstrate that it can be done with php-cs-fix.

[matej21]

@harikt please see diff with ?w=1, you have made a lot of unwanted changed (removed blank lines, added some new blank lines, renamed ArgumentOutOfRangeException to exceptions...)

[harikt]

@harikt thank you.

@dg you are welcome.

Let's convert whole nette/utils to spaces.

I am done with making to spaces. Not sure I understand you.

Now we need to convert pull requests, at least first one (#20), to demonstrate that it can be done with php-cs-fix.

That is not done with php-cs-fix . Anyway let me show how you can do that.

 /var/www/github.com/harikt/utils  ± deal   git remote add challenge-accepted https://github.com/icaine/utils
 /var/www/github.com/harikt/utils  ± deal   git fetch challenge-accepted 
remote: Counting objects: 36, done.
remote: Compressing objects: 100% (24/24), done.
remote: Total 36 (delta 20), reused 24 (delta 12)
Unpacking objects: 100% (36/36), done.
From https://github.com/icaine/utils
 * [new branch]      master     -> challenge-accepted/master
 * [new branch]      strings-cut -> challenge-accepted/strings-cut
 * [new branch]      v2.2       -> challenge-accepted/v2.2
 ✘  /var/www/github.com/harikt/utils  ± deal   git checkout strings-cut
Branch strings-cut set up to track remote branch strings-cut from challenge-accepted.
Switched to a new branch 'strings-cut'
 /var/www/github.com/harikt/utils  ± strings-cut   git merge deal 
Auto-merging src/Utils/Strings.php
CONFLICT (content): Merge conflict in src/Utils/Strings.php
Automatic merge failed; fix conflicts and then commit the result.
 ✘  /var/www/github.com/harikt/utils  ± strings-cut●✚  git mergetool
Merging:
src/Utils/Strings.php

Normal merge conflict for 'src/Utils/Strings.php':
  {local}: modified file
  {remote}: modified file
Hit return to start merge resolution tool (meld):

Before you do mergetool it is good to run the php-cs-fixer .

Here you go https://github.com/nette/utils/pull/30/files?w=1

Mostly the person who give you the PR is who do the merge. I agree for the first PR there will be conflicts. If you are trying to demonstrate me the challenges I am not here for the same. You made a deal about the move, I happily helped you.

I don't want to spend more time if you don't love it or you are trying to keep fool of the os contributors.

Thanks

[harikt]

@matej21 Thanks for pointing out that problem to me.

Normally I don't run the php-cs-fixer , but something else ;) . And I still wonder how the test pass 👎 . I can fix if needed, provided this is serious PR and @dg is not trying to make fool of me.

[dg]

challenge-accepted :-))

[vojtech-dobes]

@harikt I think your PR has green status not because tests would pass, but because it was processed by something called Scrutinizer CI, which on its build page says that Travis CI build failed. I hope I am reading it correct.

Nope, I am not :). I messed up 2 PRs into one, sorry :).

[harikt]

@dg for another note you wrote.

BTW to force Windows users to use UNIX linefeeds is exactly the same as force owner of diesel cars to pump petrol.

Have a look at https://help.github.com/articles/dealing-with-line-endings#platform-linux . Interested to hear whether it solves your issue.

Thanks!

[dg]

@harikt I try git mergetool but it let me to solve conflicts manually. But i was easy.

ad line-endings: they are converted by Git on the fly. So repo internally uses Unix line-endings (or whatever) and local files uses OS line endings, in my case Windows line-endings. Because it is nonsense to force Unix line endings on Windows.

[dg]

Btw, how to gracefully solve the rebase of branch strings-cut to master? I mean master with spaces.

[harikt]

not sure I understand your question :( .

[harikt]

Normally how are you merging things ?

[dg]

I mean git rebase http://git-scm.com/docs/git-rebase. I have in nearly all respos some dev branches and I am continually rebasing them to master, to be "on top".

[harikt]

I don't know, I am not using rebase. So my knowledge is limited.

[dg]

Rebasing is great! http://blogs.atlassian.com/2013/10/git-team-workflows-merge-or-rebase/

---

How to gracefully solve conflict with revert, for example:

git revert 77e42a0c6d4c2ee13675208307fa8992df27734e

[harikt]

ok, thanks. I will read and learn 👍 .

[harikt]

in case if you have a quick answer

git revert 77e42a0

You are checking out the commit right ? Isn't it similar to git checkout 77e42a0c6d4c2ee13675208307fa8992df27734e or may be I missed .

[harikt]

Hi @dg ,

So merge ?

[JanTvrdik]

I don't understand how is this better. This will entirely break git blame and create inconsistency with other nette/* repositories.

[dg]

@harikt rebasing (or reverting) is still not solved, it brings a lot of conflicts which I don't know how grafefully resolve. #29 (comment).

[harikt]

ok. Thank you @dg for considering. I don't want to force you if you find it doesn't have any value.

Thanks! .

[dg]

You and @pmjones said that it is good idea to switch to spaces, so I believe you will show us how to do it.

Replace tabs with spaces in repository is the most simpliest part. There is easier way than use php-cs-fixer which breaks code by replacing class ArgumentOutOfRangeException to class exception as seen here etc. This simple code do that job great:

foreach (new RecursiveIteratorIterator(new RecursiveDirectoryIterator('.')) as $file) {
    if ($file->getExtension() === 'php') {
        $s = file_get_contents($file);
        $s = str_replace("\t", '    ', $s);
        file_put_contents($file, $s);
    }
}

One thing is replacing tabs with spaces. Another thing is working with such repo. It interests me!

Every day I am using rebase, blame, sometimes revert. And now, after conversion, I don't know how to do these basic tasks. How to gracefully (i.e not being everytime confronted with conflicts) work with branches.

So @harikt and @pmjones show me please for example how to rebase this branch #20 on this (future) master https://github.com/harikt/utils/commit/5717d1eb5658b516ee19e5c85d8a23c16100e55a.

[harikt]

@dg Yes. I did it in #30 .

And this was the conversation :

DG : Ok. If Paul M. Jones will tell me that is good idea to change tabs to spaces in all my existing projects, I will promote this idea in Nette community. Deal?

And Paul did mentioned it is good. I don't want to drag again to those conversations.

I just thought it is good to help you with and did send you 2 PR. But it should not happen as a blame :-) .

[dg]

@harikt #30 is merge, not rebase. You can merge pull request once is finished (usually by „Merge pull request“ button, which will not work for older PR after tabs -> spaces conversion). But there are a lot of PR which are under development and it is common best practise to continually rebase them on master.

If I will convert tabs to spaces, everyone will ask me how to rebase his PR. What shall I answer?

[harikt]

I don't know what I should say to you. If I say, it may sound hard.

Just remember this before you make any other deals ;-) .

[xificurk]

@dg It should be possible to set up git to do the replacement for you, but I can't tell you the configuration directives from top of my head (never used them myself).

[dg]

@harikt I appreciate your help, but I need help with workflow, not with replacing strings.

I know replacing tabs with spaces makes you happy, but I am trying to show you how many real problems it causes to us. So show me, you or @pmjones, how to solve these problem. And if you don't know solutions, tell it. It may be experience for you, that tabs-conversion seems easy but is not easy. But I don't know why "to be hard".

[dg]

@xificurk tabs can be transformed on the fly with this configuration http://www.php-fg.org/

[harikt]

@dg let us drop the conversation for the good, not because I have no answer for you.

[dg]

@harikt I am really very interested in the answer.

[xificurk]

@dg This seems to cover most of the questions: https://gist.github.com/eevee/6721177

UPDATE: I did a small test following the described method - the bad news is that after the change PRs cannot be automatically merged by GitHub until they're updated (see xificurk#1), the good news is that GitHub blame is unaffected affected as well.

[hrach]

@xificurk seems pretty affected: http://cl.ly/image/2V0L453f2G3j

[xificurk]

@hrach WTF? Where did you get that? When I try blame Strings.php I see https://www.dropbox.com/s/bp1nyfsejz0mpow/blame.png?dl=0

[dg]

@xificurk scroll down

[xificurk]

omg, noob mistake :D

[dg]

I just looked at code of three well-known @php-fig members and all of them use tabs. Interesting.

(Agavi, TYPO3 and Phalcon.)

[pmjones]

Yes, the vote was roughly 2:1 in favor of spaces. The tab folk are as a result not PSR-2 compliant.

[foxycode]

I am not sure what it is going on.

@dg Do you really want to convert Nette to spaces or this is only some test?

[fprochazka]

@foxycode no, it was about demonstrating how hard is it to convert properly and how much problem it creates for maintainer and contributors

[harikt]

ie a truth :) . And as it was so hard to change Laravel, Cake 3 etc moved
to PSR-2 ;) .

Its all about willingness to accept than people trying to fool others.

Hari K T

You can ring me : +91 9388 75 8821

http://harikt.com , https://github.com/harikt ,
http://www.linkedin.com/in/harikt , http://www.xing.com/profile/Hari_KT

Skype : kthari85
Twitter : harikt

On Wed, Mar 18, 2015 at 9:24 PM, Filip Procházka [email protected]
wrote:

@foxycode https://github.com/foxycode no, it was about demonstrating
how hard is it to convert properly and how much problem it creates for
maintainer and contributors

—
Reply to this email directly or view it on GitHub
#29 (comment).

[pmjones]

Look, if you want to convert to spaces and thus be PSR-2 compliant, that's cool. If you are happy with tabs and incidentally being non-PSR-2-compliant, that's also cool. Neither is "good" or "bad" -- one is compliant, and one is not, and that's it. The tradeoffs are yours to choose between.

[dg]

@pmjones 👍

[Potherca]

With the risk of being slightly of topic (and assuming the PSR-2 compliance either doesn't happen or will take a while) why don't people just use git to convert from/to tabs/spaces? (We're on github, its safe to assume we're using git, right?)

It doesn't take much more than adding 2 config settings to a project...

Full disclosure: http://stackoverflow.com/questions/2316677/can-git-automatically-switch-between-spaces-and-tabs