All of lore.kernel.org
 help / color / mirror / Atom feed
From: Willy Tarreau <w@1wt.eu>
To: Robert Abel <rabel@robertabel.eu>
Cc: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	Andy Shevchenko <andy.shevchenko@gmail.com>
Subject: Re: [PATCH 3/4] auxdisplay: charlcd: fix x/y address commands
Date: Wed, 28 Feb 2018 05:21:34 +0100	[thread overview]
Message-ID: <20180228042134.GA27815@1wt.eu> (raw)
In-Reply-To: <911beeb5-529f-09bb-0e5a-c5b626adba60@robertabel.eu>

Hi Robert,

On Wed, Feb 28, 2018 at 12:29:38AM +0100, Robert Abel wrote:
> It is however an edge case that incurs a
> lot of code for little to no functionality.
> I'd much prefer if we broke backwards compatibility here and actually
> only parse the format that is indicated in the comment:
> 
> >         case 'x':       /* gotoxy : LxXXX[yYYY]; */
> >         case 'y':       /* gotoxy : LyYYY[xXXX]; */
> >  
> 
> Exactly one x command followed exactly by zero or one y command or
> vice-versa.
> 
> If somebody changes their mind during the escape sequence, they can just
> issue a new one instead of appending to the current one.
> 
> I'll post an example patch.

I'm sorry but I think that your patch has simply proven that your point
above doesn't stand. Adding 90 lines of code full of strchr, strpbrk and
memcpy to replace 12 trivial lines, while possibly breaking compatibility
isn't considered an improvement. Reducing code is an improvement,
multiplying it by 7 is not, it adds maintenance burden for no benefit.

Let's stick to Miguel's last version. At least now we know that the
alternatives are worse, which is great.

Thanks,
Willy

  parent reply	other threads:[~2018-02-28  4:21 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-09 23:50 [PATCH 0/3] auxdisplay: charlcd: miscellaneous patches Robert Abel
2018-02-09 23:50 ` [PATCH 1/3] auxdisplay: charlcd: fix hex literal ranges for graphics command Robert Abel
2018-02-09 23:50   ` [PATCH 2/3] auxdisplay: charlcd: use null character instead of zero literal to terminate strings Robert Abel
2018-02-09 23:50     ` [PATCH 3/3] auxdisplay: charlcd: replace octal literal with form-feed escape sequence Robert Abel
2018-02-10  9:43       ` Miguel Ojeda
2018-02-10  9:29     ` [PATCH 2/3] auxdisplay: charlcd: use null character instead of zero literal to terminate strings Miguel Ojeda
2018-02-10  8:58   ` [PATCH 1/3] auxdisplay: charlcd: fix hex literal ranges for graphics command Miguel Ojeda
2018-02-10  9:20     ` Willy Tarreau
2018-02-10  9:41       ` Miguel Ojeda
2018-02-12 13:56         ` Miguel Ojeda
2018-02-13 13:36         ` Andy Shevchenko
2018-02-13 19:15           ` Willy Tarreau
2018-02-14 23:17           ` Robert Abel
2018-02-15 10:57             ` Andy Shevchenko
2018-02-25 23:34               ` Robert Abel
2018-02-25 23:52                 ` Robert Abel
2018-02-25 23:54                 ` Robert Abel
2018-02-25 23:54                   ` [PATCH 1/4] auxdisplay: charlcd: fix two-line command ^[[LN not marked as processed Robert Abel
2018-02-25 23:54                     ` [PATCH 2/4] auxdisplay: charlcd: name x/y address struct Robert Abel
2018-02-25 23:54                       ` [PATCH 3/4] auxdisplay: charlcd: fix x/y address commands Robert Abel
2018-02-25 23:54                         ` [PATCH 4/4] auxdisplay: charlcd: make home command unshift display Robert Abel
2018-02-26 17:16                           ` Miguel Ojeda
2018-02-26  8:46                         ` [PATCH 3/4] auxdisplay: charlcd: fix x/y address commands Geert Uytterhoeven
2018-02-26 22:29                           ` Robert Abel
2018-02-26 11:44                         ` Andy Shevchenko
2018-02-26 16:54                           ` Miguel Ojeda
2018-02-26 17:09                             ` Andy Shevchenko
2018-02-26 17:26                               ` Miguel Ojeda
2018-02-26 17:56                                 ` Andy Shevchenko
2018-02-26 23:00                                 ` Robert Abel
2018-02-26 22:38                           ` Robert Abel
2018-02-26 23:06                             ` Miguel Ojeda
2018-02-26 16:49                         ` Miguel Ojeda
2018-02-26 16:57                           ` Miguel Ojeda
2018-02-26 22:43                           ` Robert Abel
2018-02-27  5:19                             ` Willy Tarreau
2018-02-27 19:31                               ` Miguel Ojeda
2018-02-27 23:29                               ` Robert Abel
2018-02-28  0:05                                 ` [PATCH RFC 0/2] auxdisplay: charlcd: fix movement and home commands Robert Abel
2018-02-28  0:05                                   ` [PATCH 1/2] auxdisplay: charlcd: fix x/y address commands Robert Abel
2018-02-28  0:05                                     ` [PATCH 2/2] auxdisplay: charlcd: make home command unshift display Robert Abel
2018-02-28  4:21                                 ` Willy Tarreau [this message]
2018-02-26 23:05                           ` [PATCH 3/4] auxdisplay: charlcd: fix x/y address commands Robert Abel
2018-02-27  5:20                             ` Willy Tarreau
2018-02-26  8:35                       ` [PATCH 2/4] auxdisplay: charlcd: name x/y address struct Geert Uytterhoeven
2018-02-26 15:59                       ` Miguel Ojeda
2018-02-26  8:34                     ` [PATCH 1/4] auxdisplay: charlcd: fix two-line command ^[[LN not marked as processed Geert Uytterhoeven
2018-02-26 15:53                       ` Miguel Ojeda
2018-02-26 23:08                   ` [PATCH 1/3] auxdisplay: charlcd: fix hex literal ranges for graphics command Robert Abel
2018-02-10 18:31     ` Geert Uytterhoeven
2018-02-10 18:58       ` Willy Tarreau

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=20180228042134.GA27815@1wt.eu \
    --to=w@1wt.eu \
    --cc=andy.shevchenko@gmail.com \
    --cc=geert@linux-m68k.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=miguel.ojeda.sandonis@gmail.com \
    --cc=rabel@robertabel.eu \
    /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.