Package home | Report new bug | New search | Development Roadmap Status: Open | Feedback | All | Closed Since Version 1.2.3

Request #12051 New "shell" engine which uses gnu diff directly via shell_exec
Submitted: 2007-09-14 01:07 UTC
From: milianw Assigned: chagenbu
Status: Closed Package: Text_Diff (version 0.2.1)
PHP Version: 5.2.3 OS: Linux
Roadmaps: (Not assigned)    
Subscription  


Anyone can comment on a bug. Have a simpler test case? Does it work for you on a different platform? Let us know! Just going to say 'Me too!'? Don't clutter the database with that please !
Your email address:
MUST BE VALID
Solve the problem : 25 + 21 = ?

 
 [2007-09-14 01:07 UTC] milianw (Milian Wolff)
Description: ------------ I've written a new Engine for this very neat PEAR package. It should be somehow similar to `string.php` but since I'm directly using `shell_exec()` it get's a little bit easier. You might ask why I've written yet another engine, well that's simple: The others were to slow. Even xdiff took like 5 seconds for approximatly 500 lines. Using GNU diff via `shell_exec()` is blazingly fast. Of course this engine wont work with `safe_mode=on`, but I think the intended audience is PHP CLI. Hope I get some feedback and maybe some bugreports. PS: This project seems dead, or are there any developers left? PPS: I hope this is the correct way to publish my engine! Test script: --------------- http://pastebin.org/2389

Comments

 [2007-09-14 04:06 UTC] chagenbu (Chuck Hagenbuch)
Not sure why you think the package is dead. Please upload the engine as a patch so we don't have to copy/paste it. Thanks!
 [2007-09-14 11:26 UTC] milianw (Milian Wolff)
Why I thought this project might be dead? Well... The last release was 2006-02-06 which is a pretty long time. But it's good, that this is not the case! I need help with creating the patch though. Is there anonymous access to the cvs repo? I can't find anything about that in the FAQ / docu. And a simple `diff -r` yields "only in Text_Diff-0.2.1-new/Diff/Engine: shell.php."
 [2007-09-14 17:19 UTC] chagenbu (Chuck Hagenbuch)
for anoncvs: http://www.php.net/anoncvs.php if all you've added is the new file just upload the raw new file.
 [2007-09-14 21:32 UTC] milianw (Milian Wolff)
I've added it.
 [2007-09-15 04:29 UTC] chagenbu (Chuck Hagenbuch)
Looks alright. A few things: - the path to the diff program should be configurable as an option to the engine - please use // or /* */ for comments instead of # - i'd like to see this use proc_open and bump the base PHP requirement to 4.3.0, instead of the mess with tempnam() and tempfiles - if you run the diff.phpt test using this engine it doesn't pass.
 [2007-09-15 13:53 UTC] milianw (Milian Wolff)
> - the path to the diff program should be configurable as > an option to the engine > - please use // or /* */ for comments instead of # Done. > - if you run the diff.phpt test using this engine it > doesn't pass. Fixed, forgot the `trimNewlines`. > - i'd like to see this use proc_open and bump the base > PHP requirement to 4.3.0, instead of the mess with > tempnam() and tempfiles I see that requireing temporary files could be an issue for using this engine on different systems (e.g. unix vs. windows). But I don't get it how I should use proc_open here. Maybe I misunderstand it, but wouldn't I need to call diff so that it reads from stdin? I can't find such an option for it. Maybe you could give me a little help here, to get started?
 [2007-09-15 22:37 UTC] chagenbu (Chuck Hagenbuch)
Mmm, I might not have thought that through on using proc_open. Looks pretty good and the test passes; needs a little bit of cleanup but I'll do that in the next day or two and get it committed.
 [2007-09-15 22:52 UTC] milianw (Milian Wolff)
Great! I hope I'll get notified so that I can see which parts needed cleanup.
 [2007-09-15 22:56 UTC] chagenbu (Chuck Hagenbuch)
Sure - I'll update this ticket, and this is where the file will go when it's checked in: http://cvs.horde.org/framework/Text_Diff/Diff/Engine/
 [2007-09-20 21:13 UTC] milianw (Milian Wolff)
I've found a little bug which was found by the assert. I've updated the file. Ouh, but I forgot to remove the "or die();" after the assert. Don't think that should be kept - please remove it. The real "changed" lines are those below. See: http://pear.php.net/bugs/patch-display.php?patch=shell.php&bug=12051&diff=1&old=1189864020&revision=1190322631
 [2007-09-21 00:37 UTC] chagenbu (Chuck Hagenbuch)
This has been committed and I released 0.3.0. This package is maintained mostly be Horde developers, where the package is hosted and where it originated (http://cvs.horde.org/framework/Text_Diff/). Would you be willing to assign your copyright on that engine to Horde so that we can deal more easily with the package as a whole? Thanks!
 [2007-09-21 12:09 UTC] milianw (Milian Wolff)
Of course, do as you please. But it would be nice if at least the "@author" line could be kept. I will write a test case for the engine today. Hope that everything is alright, now that you released a new version!
 [2007-09-21 16:09 UTC] chagenbu (Chuck Hagenbuch)
We'll absolutely keep the @author line and any other contact/credit information you'd like (within reason :). Thanks!
 [2007-09-25 00:10 UTC] milianw (Milian Wolff)
I've submitted a new patch (against CVS head). It should finally be correct, the old version had still some bugs. Please give it a try and have a look into the source since I might have messed some formatting up again (i.e. whitespaces / uppercase comments). Thanks and have a good night
 [2007-09-25 00:26 UTC] milianw (Milian Wolff)
The following still produces wrong diff, so dont merge yet! $str1 = 'a a a a x x bc x x '; $str2 = 'x c c c x ba x x ';
 [2007-09-25 03:17 UTC] chagenbu (Chuck Hagenbuch)
Okay. By the way, have you looked at the string engine? It is already set up to parse the output of diff-style programs, so you might be able to piggyback on that.
 [2007-09-25 14:04 UTC] milianw (Milian Wolff)
Finally, this seems to be it. See the submitted `shell-engine-0.3.1.patch` (newest revision).
 [2007-09-25 14:07 UTC] milianw (Milian Wolff)
PS: Yes, I had a look at the `string.php` engine a while ago, though it does no seem to support the standard diff output, does it? I didn't find anything like (a|c|d) or even (< |> ) in it. Did'nt try it out though!
 [2007-09-25 21:15 UTC] chagenbu (Chuck Hagenbuch)
Committed, thanks! I'll look at this vs. string.php at some point.