Hello Junio, * Junio C Hamano wrote on Tue, Nov 06, 2007 at 09:46:35PM CET: > All missing Signed-off-by: lines. Oops. Sorry. > [1/5] In addition to take advantage of the fact that the RHS of > assignment is not split, I'd prefer replacing `` with $() > with these cases. Much easier to read if your shell > supports it (and all the modern ones do). OK. > [2/5] Gaah, AIX sed X-<. I am not opposed to this patch but > would want to get Yays from people with non GNU sed. Is > busybox sed good enough to grok our scripts these days? > Please ask help and collect Acks at least from folks on > Solaris, MacOS, FBSD, and OBSD. FWIW, I have little experience with busybox sed, but for the others here you go: With echo axbyc | sed 's,x,\n,; s,y,\ ,' I get on OpenBSD, FreeBSD, Solaris, and Darwin (minus indentation): anb c GNU sed gives a b c > [3/5] Arithmetic expansion. Have you caught _all_ of them, or > is this patch about only the ones you noticed? I have grepped *.sh. But let's drop that, I see that it goes backwards. > [4/5] I wonder if use of fgrep would be easier to read and more > portable with this one: > > name=$( GIT_CONFIG=.gitmodules \ > git config --get-regexp '^submodule\..*\.path$' | > fgrep "submodule.$1.path" | > sed -e 's/^submodule\.\(.*\)\.path$/\1/' > ) Certainly easier to read. But fgrep itself is not portable (it could be grep -F). Also, isn't the $1 to be matched at the end, after a "=" here? FWIW the pattern I posted has survived a few years in Automake, so there is some hope that it works. > [5/5] Again, have you covered all of them? No, oops again. As I searched for `test.*-[oa]' I have missed line wraps and [ ... -o ... ]. > I am not opposed to > this one, although I am a bit curious who lacks -a/-o in > practice. Hmm, good question. I actually don't know whether there is a shell that isn't ruled out by $() anyway. Let's drop that one, too, then. Cheers, Ralf - To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
