All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sidong Yang <realwakka@gmail.com>
To: dsterba@suse.cz, linux-btrfs <linux-btrfs@vger.kernel.org>,
	Filipe Manana <fdmanana@gmail.com>
Subject: Re: [RFC] btrfs-progs: format-output: remove newline in fmt_end text mode
Date: Sat, 20 Feb 2021 16:42:38 +0000	[thread overview]
Message-ID: <20210220164238.GB11258@realwakka> (raw)
In-Reply-To: <20210219215611.GM1993@twin.jikos.cz>

On Fri, Feb 19, 2021 at 10:56:11PM +0100, David Sterba wrote:
> On Tue, Feb 16, 2021 at 04:28:40PM +0000, Sidong Yang wrote:
> > Remove a code that inserting new line in fmt_end() for text mode.
> > Old code made a failure in fstest btrfs/006.
> > 
> > Signed-off-by: Sidong Yang <realwakka@gmail.com>
> > ---
> > Hi, I've just read mail that Filipe written that some failure about fstest.
> > I'm worried about this patch makes other problem. So make it RFC. Thanks.
> 
> I found the discussion under the device stats patch adding json, the
> added line was known and "hopefully not causing problems", but the
> fstests seem to notice.
> 
> I think we can fix that by removing the fmt_end newline but we also need
> to update how the fmt_print is done for the text output. Ie. for json
> there are some strict rules for line continuations  (",") but for the
> textual output, each line ended by "\n" right away, without delaying
> that to the next fmt_* call should work.

You mean that if this patch applied and the code prints device stats for
text format manually replaced to fmt_print(), there is no last new line
for text output? fmt_print() prints new line before print some value now.
I think that it should prints new line at the end of each fmt_print().
like below

diff --git a/common/format-output.c b/common/format-output.c
index f5b12548..9a9f5bf7 100644
--- a/common/format-output.c
+++ b/common/format-output.c
@@ -242,7 +239,6 @@ void fmt_print(struct format_ctx *fctx, const char* key, ...)
                const bool print_colon = row->out_text[0];
                int len;
 
-               putchar('\n');
                fmt_indent1(fctx->indent);
                len = strlen(row->out_text);
 
@@ -312,6 +308,8 @@ void fmt_print(struct format_ctx *fctx, const char* key, ...)
        }
 
        fmt_end_value(fctx, row);
+       if (bconf.output_format == CMD_FORMAT_TEXT)
+               putchar('\n');
        /* No newline here, the line is closed by next value or group end */
        va_end(args);
 }

Thanks,
Sidong

  reply	other threads:[~2021-02-20 16:44 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-16 16:28 [RFC] btrfs-progs: format-output: remove newline in fmt_end text mode Sidong Yang
2021-02-17 10:57 ` Filipe Manana
2021-02-19 21:56 ` David Sterba
2021-02-20 16:42   ` Sidong Yang [this message]
2021-02-22 15:46     ` David Sterba

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210220164238.GB11258@realwakka \
    --to=realwakka@gmail.com \
    --cc=dsterba@suse.cz \
    --cc=fdmanana@gmail.com \
    --cc=linux-btrfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.