Re: [PATCH 2/2] kconfig: echo stdin to stdout if either is redirected

[Ulf Magnusson]

On Thu, Feb 8, 2018 at 6:56 AM, Masahiro Yamada
<yamada.masahiro@xxxxxxxxxxxxx> wrote:
> If stdio is not tty, conf_askvalue() puts additional new line to
> prevent prompts are all concatenated into a single line.  This care
> is missing in conf_choice(), so a 'choice' prompt and the next prompt
> are shown in the same line.
>
> Move the code into xfgets() to take care of all cases.  To improve
> this more, echo stdin to stdout.  This clarifies what keys were input
> from stdio and the stdout looks like as if it were from tty.
>
> I removed the isatty(2) check since stderr is unrelated here.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx>
> ---
>
>  scripts/kconfig/conf.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
> index 358e2e4..c5318d3 100644
> --- a/scripts/kconfig/conf.c
> +++ b/scripts/kconfig/conf.c
> @@ -76,6 +76,9 @@ static void xfgets(char *str, int size, FILE *in)
>  {
>         if (!fgets(str, size, in))
>                 fprintf(stderr, "\nError in reading or end of file.\n");
> +
> +       if (!tty_stdio)
> +               printf("%s", str);
>  }
>
>  static int conf_askvalue(struct symbol *sym, const char *def)
> @@ -106,8 +109,6 @@ static int conf_askvalue(struct symbol *sym, const char *def)
>         case oldaskconfig:
>                 fflush(stdout);
>                 xfgets(line, sizeof(line), stdin);
> -               if (!tty_stdio)
> -                       printf("\n");
>                 return 1;
>         default:
>                 break;
> @@ -495,7 +496,7 @@ int main(int ac, char **av)
>         bindtextdomain(PACKAGE, LOCALEDIR);
>         textdomain(PACKAGE);
>
> -       tty_stdio = isatty(0) && isatty(1) && isatty(2);
> +       tty_stdio = isatty(0) && isatty(1);
>
>         while ((opt = getopt_long(ac, av, "s", long_opts, NULL)) != -1) {
>                 if (opt == 's') {
> --
> 2.7.4
>

Reviewed-by: Ulf Magnusson <ulfalizer@xxxxxxxxx>

Might be some case I'm not thinking of, but wouldn't it make sense to
just check isatty(1) as well? If stdout is a regular file, it seems
it'd be nice to have the input appear there, regardless of where stdin
is from.

Maybe the tty_stdio variable could be removed then as well, replaced
with just 'if (!isatty(1))'.

Cheers,
Ulf