On Thu, 2015-04-16 at 10:16 -0700, Junio C Hamano wrote:
> Jeff King <p...@peff.net> writes:
> 
> > On Thu, Apr 16, 2015 at 08:39:55AM -0700, Junio C Hamano wrote:
> >
> >> >  test_expect_success 'status untracked directory with --ignored' '
> >> >          echo "ignored" >.gitignore &&
> >> > +        sed -e "s/^/\xef\xbb\xbf/" .gitignore >.gitignore.new &&
> >> > +        mv .gitignore.new .gitignore &&
> >> 
> >> Is this "write literal in \xHEX on the replacement side of sed
> >> substitution" potable?  In any case, replacing the above three with
> >> something like:
> >> 
> >>    printf "<bom>ignored\n" >.gitignore
> >> 
> >> may be more sensible, no?
> >
> > I'm not sure about sed, but I agree it is suspect. And note that printf
> > with hex codes is not portable, either You have to use octal:
> >
> >   printf '\357\273\277ignored\n' >.gitignore
> >
> > Also, as a nit, I'd much rather see this in its own test rather than
> > crammed into another test_expect_success. It's much easier to diagnose
> > failures if the test description mentions the goal, and it is not tied
> > up with testing other parts that might fail.
> 
> Yeah, I totally agree.
> 
> Carlos, something like this squashed in, perhaps?
> 
>  t/t7061-wtstatus-ignore.sh | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/t/t7061-wtstatus-ignore.sh b/t/t7061-wtstatus-ignore.sh
> index 0a06fbf..cdc0747 100755
> --- a/t/t7061-wtstatus-ignore.sh
> +++ b/t/t7061-wtstatus-ignore.sh
> @@ -13,8 +13,6 @@ EOF
>  
>  test_expect_success 'status untracked directory with --ignored' '
>       echo "ignored" >.gitignore &&
> -     sed -e "s/^/\xef\xbb\xbf/" .gitignore >.gitignore.new &&
> -     mv .gitignore.new .gitignore &&
>       mkdir untracked &&
>       : >untracked/ignored &&
>       : >untracked/uncommitted &&
> @@ -22,6 +20,15 @@ test_expect_success 'status untracked directory with 
> --ignored' '
>       test_cmp expected actual
>  '
>  
> +test_expect_success 'same with gitignore starting with BOM' '
> +     printf "\357\273\277ignored\n" >.gitignore &&
> +     mkdir -p untracked &&
> +     : >untracked/ignored &&
> +     : >untracked/uncommitted &&
> +     git status --porcelain --ignored >actual &&
> +     test_cmp expected actual
> +'
> +
>  cat >expected <<\EOF
>  ?? .gitignore
>  ?? actual
> 

Yeah, that makes sense. I had something similar in my patch at one point
before going with modifying the current one.

   cmn


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to