You will find attached several patches written by Jörg Sommer <joerg [at] alea.gnuu.de> which has improved the code.
Patches from Jörg Sommer
Thanks. After a quick scan I think 1, 2 and 3 can be tested but for instance patch 4 may not be compatible with some OSes (AFAIK not all 'expr' have "match") and patch 6 is not Bourne compatible. (Deities know I would be the *first* one to allow myself the benefits of BAsh-isms). In any case we'll definately test these patches.
Thanks for your work!
Patch 3 is wrong. Under Solaris 'which' return code does not state whether the command exists or not.
The patch must not be applied.
Patch 2 cannot be applied since Solaris grep has no '-q' option. The patch must not be applied.
As with unSpawn patch 1 seems fine.
Patch 4 may be possible. It works under Solaris, but would need testing on BSD at least (I have openbsd, and soon netbsd again at work). I would however perhaps suggest using the ':' convention rather than 'match' since ':' seems to be more known.
Patch 5 is also possible, but I see no real advantage except perhaps in the display and suspscan functions. The fact that 1 pipe is/is not used in a command sequence is not going to be noticed by anyone. A pipe in a loop that is executed many times would be more of a consideration - the display function is executed many times as a whole, whereas in suspscan the use of sed in a loop may be worthwhile.
Patch 6 would not normally work. However, since the change is only made in the BSD section, it is possible that it could be used.
As unSpawn said, some patches are possible but would require testing under other OSes.
Patch 4: As suspected we would have to use the ':' operator rather than 'match'. Openbsd does not like match.
Thanks again for your work.
To sum up:
* Patch 1 can be applied
* Patches 2 & 3 cannot as they do not work on Solaris
* Patch 4 needs to be amended to use ':' as match is not everywhere
* Patch 5 is ok but not very useful ATM - I think however it should be applied, don't you?
* Patch 6 needs to be test on BSD
Patch 1 applied, testing locally before committing. Should be OK though.
Patch 2 rejected: Solaris.
Patch 3 rejected: Solaris.
Patch 4 rejected: 'echo' being a builtin in Bash, Ksh (and Jsh, Ash, Bsh) so only benefit is where it's not (Tcsh, Csh?). It also fails to patch --fuzz cleanly.
Patch 5 rejected: not a major improvement in terms of speed or otherwise.
Patch 6 rejected: fails Heirloom* Sh: no variable substring ops. It also fails to patch --fuzz cleanly.
* Gunnar Ritter, strict SVR4/SVID3 level shell, used for compatibility testing.
Thanks for submitting the patches. Commit done.
Log in to post a comment.