vicoapp / vico

Mac Programmers Text Editor
http://www.vicoapp.com/
658 stars 89 forks source link

Indentation rules broken in PHP #22

Closed rtgibbons closed 11 years ago

rtgibbons commented 12 years ago

I haven't found all the cases, but it happens multi-line arrays. ex.

I'd expect the code to look like this after re-indenting with =} or gg=G

<?php
$temp = array(
    'multi',
    'line',
    array(
        'arrays',
        'break'
        ),
    'indentation'
)
?>

instead the formatted code appears as

<?php
$temp = array(
    'multi',
    'line',
    array(
        'arrays',
        'break'
        ),
        'indentation'
            )
                ?>
Shadowfiend commented 12 years ago

Out of curiosity, do you know if this same thing happens in TextMate? I ask because this seems to be based οff of the TextMate PHP bundle, so if it also happens in TextMate then it would seem that that's what's broken, while if it only happens in vico it would seem vico's interpretation of the tmbundle stuff is what's broken.

rtgibbons commented 12 years ago

I had to download TM (never used it) TextMate 1 - Re-Indent (Text -> Indent Selection) produces the same output as Vico. So it seems to be a problem with the bundle

rtgibbons commented 12 years ago

Look like the tmbundle's should be updated, at least with the PHP one, this bug was fixed https://github.com/textmate/php.tmbundle/commit/a6738814e36e5175536f9dde884023d62e8d73cb

Shadowfiend commented 12 years ago

Got it. I think that means @martinh just has to pull those changes into the vico php.tmbundle and then update the submodule in this repository.

Shadowfiend commented 11 years ago

I'll try and get to this soon.

Shadowfiend commented 11 years ago

@rtgibbons, I've synchronized Vico's php.tmbundle with the textmate-1.x branch of the Textmate one, which is at commit a673881 as you indicated. It doesn't seem to have worked, but if you could give it a whirl (the change should be on the master branch of this repository) and let me know if things do seem fixed, that'd be cool. Thanks!

rtgibbons commented 11 years ago

I haven't used Vico in a while, since it was announced "Dead" I'll get a build and try it out again.

rtgibbons commented 11 years ago

Still doesn't indent correct, using the code sample above.

Shadowfiend commented 11 years ago

Yeah, that's what I thought, just wanted to double-check.

Does TextMate indent it correctly with that version of the bundle? Had you verified that? I couldn't tell for sure.

rtgibbons commented 11 years ago

I don't have a Textmate license to test with :/

Shadowfiend commented 11 years ago

Fair enough, thanks. I'll see if I can do it with a trial license sometime soon and find out if that's an issue with the bundle or with Vico, and prioritize the fix accordingly.

weslly commented 11 years ago

FWIW, textmate 2 doesn't require a license at this time. http://api.textmate.org/downloads/release

Shadowfiend commented 11 years ago

Cool; I'll play with both, but I suspect Textmate 2 has at least modified the bundle structure, so we'll have to see if that will help or not.

Shadowfiend commented 11 years ago

Aha! ;) Well if it's a bundle issue, let me see what I can wrangle!

Shadowfiend commented 11 years ago

This indentation should now work on master. Let me know how it looks. So should the # issue reported in the above linked issue. The other example presented there is a different matter, I dunno what's up there.

Shadowfiend commented 11 years ago

Nm, fixed the other example as well.

Shadowfiend commented 11 years ago

Ok, I'm going to wait for some indication that this is fixed for a couple of days and close the issue if I don't hear otherwise. Let me know!

Shadowfiend commented 11 years ago

Note: the easiest way to test this would be to download the installer at the “Product” link on the Vico build bot to install a version of Vico built from latest master, which should include the latest Vico PHP bundle tweaks.

berliner commented 11 years ago

Hey, I would like to test this, but I use TextMate 2 and do not want to install additional software. Did you change the bundle or some logic proper to vico?

Shadowfiend commented 11 years ago

I changed the bundle, but the repo where that is done is based on the TextMate 1 bundle format, not that of TM2, I'm afraid. I added https://github.com/vicoapp/php.tmbundle/commit/615ad2bfcba393eb2fe35f9b850a3d96800f5fc0 and https://github.com/vicoapp/php.tmbundle/commit/59c24cf26b30d794b098876f94d6e0e6bf3150fa to the bundle we're tracking. The TM2 bundle already had some measure of support for multi-dimensional arrays and fixed the # issue. I fixed indentation in the face of empty curlies ({}) and another aspect of multi-dimensional array indentation.

Shadowfiend commented 11 years ago

I'm going to go ahead and close this, though I also am going to import the (differences in the fix that the tmbundle folks made)[https://github.com/textmate/php.tmbundle/commit/901e9ca38a87dc18f11f3dce9a3f5eee01cb7b0d], if they apply, when I get a chance.