qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: "Daniel P. Berrangé" <berrange@redhat.com>,
	"Eduardo Habkost" <ehabkost@redhat.com>,
	qemu-block@nongnu.org, "Michael S. Tsirkin" <mst@redhat.com>,
	"Jason Wang" <jasowang@redhat.com>,
	"Juan Quintela" <quintela@redhat.com>,
	"Yuval Shaia" <yuval.shaia.ml@gmail.com>,
	qemu-devel@nongnu.org, "Gerd Hoffmann" <kraxel@redhat.com>,
	"Andrea Bolognani" <abologna@redhat.com>,
	"Marc-André Lureau" <marcandre.lureau@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Michael Roth" <mdroth@linux.vnet.ibm.com>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>,
	"Stefan Berger" <stefanb@linux.ibm.com>
Subject: Re: [PATCH] schemas: Add vim modeline
Date: Fri, 31 Jul 2020 13:26:48 +0200	[thread overview]
Message-ID: <20200731112648.GE4707@linux.fritz.box> (raw)
In-Reply-To: <875za4t5vw.fsf@dusky.pond.sub.org>

Am 31.07.2020 um 11:01 hat Markus Armbruster geschrieben:
> Kevin Wolf <kwolf@redhat.com> writes:
> 
> > Am 30.07.2020 um 17:11 hat Eric Blake geschrieben:
> >> > JSON is a exceptionally poor choice for a DSL, or even a configuration
> >> > language.
> >> > 
> >> > Correcting our mistake involves a flag day and a re-learn.  We need to
> >> > weigh costs against benefits.
> >> > 
> >> > The QAPI schema language has two layers:
> >> > 
> >> > * JSON, with a lexical and a syntactical sub-layer (both in parser.py)
> 
> An incompatible dialect of JSON with a doc comment language, actually.
> 
> The need to keep doc generation working could complicate replacing the
> lower layer.

Good point, we would have to keep the comment parser either way to be
used on top of the YAML (or whatever) parser.

Whatever parser we use would have to actually make comments available
rather than immediately filtering them out. This might exist for most
languages, but it will probably not be the most commonly used parser
either (or at least it will not allow using a simple interface like
json.loads() in Python).

> >> > 
> >> > * QAPI, with a context-free and a context-dependend sub-layer (in
> >> >    expr.py and schema.py, respectively)
> >> > 
> >> > Replacing the JSON layer is possible as long as the replacement is
> >> > sufficiently expressive (not a tall order).
> >> 
> >> I'm open to the idea, if we want to attempt it, and agree with the
> >> assessment that it is not a tall order.
> 
> Careful, "not a tall order" is meant to apply to the "sufficiently
> expressive" requirement for a replacemnt syntax.
> 
> On actually replacing the lower layer, I wrote "we need to weigh costs
> against benefits."
> 
> > I'm not so sure about that. I mean, it certainly sounds doable if need
> > be, but getting better syntax highlighting by default in some editors
> > feels like a pretty weak reason to switch out the complete schema
> > language.
> >
> > At first I was going to say "but if you don't have anything else to do
> > with your time...", but it's actually not only your time, but the time
> > of everyone who has development branches or downstream repositories and
> > will suffer rather nasty merge conflicts. So this will likely end up
> > having a non-negligible cost.
> 
> Yup.
> 
> > So is there more to it or are we really considering doing this just
> > because editors can tell more easily what to do with a different syntax?
> 
> If memory serves, the following arguments have been raised:
> 
> 1. A chance to improve ergonomics for developers
> 
>    Pain points include
> 
>    - Confusion
> 
>      It claims to be JSON, but it's not.
> 
>    - Need to learn another syntax
> 
>      Sunk cost for old hands, but it's a valid point all the same.

We use a similar (same?) form of "almost JSON" for QMP which will still
exist. So we'd be moving from having to learn one (non-standard)
language to two languages (one still non-standard and another one that
is hopefully more standard).

>    - Poor tool support
> 
>      JSON tools don't work.  Python tools do, but you may have to work
>      around the issue of true, false.

This is mostly the editor question this patch was about, right? Or are
people trying to use more sophisticated tools on it?

>    - Excessive quoting
> 
>    - Verbosity
> 
>      When all you have is KEY: VALUE, defining things with multiple
>      properties becomes verbose like
> 
>          'status': { 'type': 'DirtyBitmapStatus',
>                      'features': [ 'deprecated' ] }
> 
>      We need syntactic sugar to keep vebosity in check for the most
>      common cases.  More complexity.

I don't think this is something any of the suggested languages would
address.

>    - No trailing comma in arrays and objects
> 
>    - No way to split long strings for legibility
> 
>    - The doc comment language is poorly specified
> 
>    - Parse error reporting could be better (JSON part) / could hardly be
>      worse (doc comment part)

Has anyone looked into what error messages are like for the suggested
alternatives? "error reporting could be better" is something that is
true for a lot of software.

The doc comment part is not going to change unless we get rid of
comments and actually make documentation part of the objects themselves.
This might not be very readable.

Or I should rather say, making the doc comment part change is possible,
but would require the same changes as with our current lower layer
language and parser.

> 2. Not having to maintain our own code for the lower layer
> 
>    I consider this argument quite weak.  parser.py has some 400 SLOC.
>    Writing and rewriting it is sunk cost.  Keeping it working has been
>    cheap.  Keeping the glue for some off-the-shelf parser working isn't
>    free, either.  No big savings to be had here, sorry.
> 
>    Almost half of parser.c is about doc comments, and it's the hairier
>    part by far.  Peter has patches to drag the doc comment language
>    closer to rST.  I don't remember whether they shrink parser.py.
> 
> 3. Make the schema more easily consumable by other programs
> 
>    Use of a "standard" syntax instead of our funky dialect of JSON means
>    other programs can use an off-the-shelf parser instead of using or
>    reimplementing parser.py.
> 
>    Valid point for programs that parse the lower layer, and no more, say
>    for basic syntax highlighting.
> 
>    Pretty much irrelevant for programs that need to go beyond the lower
>    layer.  Parsing the lower layer is the easy part.  The code dealing
>    with the upper layer is much larger (expr.py and schema.py), and it
>    actually changes as we add features to the schema language.
>    Duplicating it would be a Bad Idea.  Reuse the existing frontend
>    instead.

Do other programs that go beyond syntax highlighting actually get to
parse our QAPI schema definitions? Or would they rather deal with the
return value of query-qmp-schema?

Neither the QAPI schema nor a YAML file with the same structure are a
standard approach to describe JSON documents. So even if we replace JSON
in the lower layer, the whole thing (and as you say, the upper layer is
the more interesting part) still stays non-standard in a way and more
advanced tools can't be used with it.

And of course, even if we did use something more standard like JSON
Schema or whatever exists for YAML, we would still have to massively
extend it because the QAPI schema contains much more information than
just what would be needed to validate some input. We control all aspects
of generated C code with it.

Kevin



  reply	other threads:[~2020-07-31 11:28 UTC|newest]

Thread overview: 85+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-29 18:50 [PATCH] schemas: Add vim modeline Andrea Bolognani
2020-07-30  9:07 ` Markus Armbruster
2020-07-30  9:37   ` Daniel P. Berrangé
2020-07-30 11:51     ` Markus Armbruster
2020-07-30 13:24       ` Daniel P. Berrangé
2020-07-31  6:45         ` John Snow
2020-07-31  9:21           ` Markus Armbruster
2020-07-31  9:32             ` Daniel P. Berrangé
2020-07-31 12:55         ` Markus Armbruster
2020-07-31 15:07           ` Daniel P. Berrangé
2020-07-31 15:26             ` John Snow
2020-07-31 15:44               ` Daniel P. Berrangé
2020-08-03  7:28                 ` Markus Armbruster
2020-08-03  8:41                 ` Paolo Bonzini
2020-08-03 11:24                   ` Markus Armbruster
2020-08-03 11:36                   ` Daniel P. Berrangé
2020-08-03 12:16                     ` Paolo Bonzini
2020-08-03 12:23                       ` Daniel P. Berrangé
2020-08-03 12:33                         ` Paolo Bonzini
2020-08-03 12:43                           ` Daniel P. Berrangé
2020-08-03 15:48                         ` Markus Armbruster
2020-08-03 21:02                         ` Nir Soffer
2020-07-31 16:35               ` Paolo Bonzini
2020-07-31 16:41                 ` Dr. David Alan Gilbert
2020-07-31 17:20                 ` Daniel P. Berrangé
2020-07-31 17:47                   ` Paolo Bonzini
2020-08-03  9:44                     ` Daniel P. Berrangé
2020-07-31 17:53                 ` John Snow
2020-07-31 18:01                   ` Paolo Bonzini
2020-08-03  7:45                     ` Markus Armbruster
2020-07-31 16:28             ` cleanups with long-term benefits (was Re: [PATCH] schemas: Add vim modeline) Paolo Bonzini
2020-07-31 17:05               ` Daniel P. Berrangé
2020-07-31 17:16                 ` Paolo Bonzini
2020-07-31 17:27                   ` Daniel P. Berrangé
2020-07-31 17:42                     ` Paolo Bonzini
2020-08-03  9:27                       ` Daniel P. Berrangé
2020-08-03  8:18               ` Markus Armbruster
2020-08-03  8:42                 ` Paolo Bonzini
2020-08-03 11:28                   ` Markus Armbruster
2020-08-03 12:01                     ` Paolo Bonzini
2020-08-03 16:03                       ` Markus Armbruster
2020-08-03 16:36                         ` Kevin Wolf
2020-08-04  7:28                           ` Markus Armbruster
2020-08-03 17:19                         ` Paolo Bonzini
2020-08-04  8:03                           ` Markus Armbruster
2020-08-04 18:24                             ` John Snow
2020-08-05  7:36                               ` Markus Armbruster
2020-08-05  8:25                                 ` Paolo Bonzini
2020-08-05  8:39                                   ` Dr. David Alan Gilbert
2020-08-05  8:49                                     ` Paolo Bonzini
2020-08-05  9:05                                       ` Daniel P. Berrangé
2020-08-05  9:11                                         ` cleanups with long-term benefits Cornelia Huck
2020-08-05 10:08                                           ` Daniel P. Berrangé
2020-08-05 10:24                                             ` Cornelia Huck
2020-08-05 16:23                                             ` Kevin Wolf
2020-08-05 16:46                                               ` Eduardo Habkost
2020-08-06  5:44                                                 ` Markus Armbruster
2020-08-05  8:47                                   ` Cornelia Huck
2020-08-05  8:56                                   ` cleanups with long-term benefits (was Re: [PATCH] schemas: Add vim modeline) Markus Armbruster
2020-08-05 10:15                                     ` Alex Bennée
2020-08-05 16:04                                 ` John Snow
2020-08-06  4:58                                   ` Markus Armbruster
2020-08-05  8:42                         ` Markus Armbruster
2020-08-03 18:10                       ` John Snow
2020-08-03 18:16                         ` Paolo Bonzini
2020-08-03 18:19                           ` John Snow
2020-08-03 19:54                             ` Nir Soffer
2020-08-03 20:48                               ` John Snow
2020-08-03  9:50                 ` Daniel P. Berrangé
2020-08-03 11:32                   ` Markus Armbruster
2020-07-31 16:39             ` [PATCH] schemas: Add vim modeline Kevin Wolf
2020-07-30 15:11       ` Eric Blake
2020-07-30 20:53         ` John Snow
2020-07-30 20:56         ` John Snow
2020-07-31  7:15         ` Kevin Wolf
2020-07-31  8:48           ` Daniel P. Berrangé
2020-07-31  9:01           ` Markus Armbruster
2020-07-31 11:26             ` Kevin Wolf [this message]
2020-08-03  8:51               ` Markus Armbruster
2020-07-31 23:12     ` Nir Soffer
2020-08-03 12:16       ` Paolo Bonzini
2020-08-04  7:28         ` Markus Armbruster
2020-08-04  8:29     ` Alex Bennée
2020-09-07 13:54   ` Markus Armbruster
2020-07-30 13:14 ` Markus Armbruster

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=20200731112648.GE4707@linux.fritz.box \
    --to=kwolf@redhat.com \
    --cc=abologna@redhat.com \
    --cc=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=jasowang@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=quintela@redhat.com \
    --cc=stefanb@linux.ibm.com \
    --cc=yuval.shaia.ml@gmail.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).