Re: [Development] Support for *Notes and UpstreamFiles fields in qt_attributions.json files
> -Original Message- > From: Development On Behalf Of > Hasselmann Mathias via Development > Subject: Re: [Development] Support for *Notes and UpstreamFiles fields in > qt_attributions.json files > > Hi, > > Just to make ensure all options are considered: How about the elephant in the > room? > How about "simply" implementing JSONC (JSON with Comments) in Qt's JSON > parser instead? > > * People wonder regularly when they learnm that there are no comments in > JSON. > * JSONC is used by popular software like VSCode and TypeScript. > * The runtime parsers of PowerShell, .NET, and probably others already > support > it out of the box. For JavaScript and Python there are drop-in libraries. > * A MIT licensed implementation with examples and tests can be found here: > https://github.com/Microsoft/node-jsonc-parser Supporting this in QJson would be nice, indeed. This could be an additional mode, like described in https://bugreports.qt.io/browse/QTBUG-44226 . Anyhow, given that this wasn't high priority so far, I wouldn't like to wait for it. Even if we get such a JSON mod at one point, there is arguably little harm in having the extra Comment field. So, to wrap up: Let's go for the Comment field, as this fixes the immediate need. Extending our JSON support to allow comments, or even switching to an alternative file system is something we can still consider mid-term. Regards Kai -- Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
Re: [Development] Support for *Notes and UpstreamFiles fields in qt_attributions.json files
Hi,
Just to make ensure all options are considered: How about the elephant
in the room?
How about "simply" implementing JSONC (JSON with Comments) in Qt's JSON
parser instead?
* People wonder regularly when they learnm that there are no comments in
JSON.
* JSONC is used by popular software like VSCode and TypeScript.
* The runtime parsers of PowerShell, .NET, and probably others already
support
it out of the box. For JavaScript and Python there are drop-in libraries.
* A MIT licensed implementation with examples and tests can be found here:
https://github.com/Microsoft/node-jsonc-parser
Ciao
Mathias
Am 16.02.2023 um 10:57 schrieb Edward Welbourne via Development:
Kai Köhne (15 February 2023 08:50) replied:
Well, you can also achieve this by duplicating comment fields:
{
"Comment": "Upstream files are src/x.cpp, include/y.h",
"Files": [ "x.cpp", "y_p.h"]
"Comment": "Copyright info is from dist/COPYING",
"Copyright": "Copyright (C) 2023 Joe Doe"
}
Edward Welbourne (Wednesday, February 15, 2023 10:45 AM) objected:
The problem with that is that I was given to understand that
duplicated keys is actually malformed JSON - perhaps I misunderstood.
If that's legitimate JSON, then I'm fine with just one.
and, overlapping with my follow-up correcting that,
Kai Köhne (15 February 2023 17:10) replied:
To my understanding it's valid JSON, at least from the syntax
side. From
https://www.ecma-international.org/publications-and-standards/standards/ecma-404/
:
The JSON syntax does not impose any restrictions on the strings used
as names, *does not require that name strings be unique*, and does
not assign any significance to the ordering of name/value
pairs. These are all semantic considerations that may be defined by
JSON processors or in specifications defining specific uses of JSON
for data interchange
It seems we read the same standard at about the same time, arriving at
the same conclusion ;^>
And the JSON parsers I tested (Python, Qt) don't treat it as an error,
either. There seems to be some online linters like
https://jsonlint.com/ that complain about it, tough.
I think we can live with ignoring a warning from linters that aren't
part of our tool-chain ;^>
I've updated my reviews to the "Comment" version.
That just leaves us with the open question of whether to, instead,
switch to some other way of storing the data - preferably one that
supports multi-line strings - with Ulf currently championing QML and
Thiago YAML.
Eddy.
--
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development
Re: [Development] Support for *Notes and UpstreamFiles fields in qt_attributions.json files
Kai Köhne (15 February 2023 08:50) replied:
>>> Well, you can also achieve this by duplicating comment fields:
>>>
>>> {
>>> "Comment": "Upstream files are src/x.cpp, include/y.h",
>>> "Files": [ "x.cpp", "y_p.h"]
>>> "Comment": "Copyright info is from dist/COPYING",
>>> "Copyright": "Copyright (C) 2023 Joe Doe"
>>> }
Edward Welbourne (Wednesday, February 15, 2023 10:45 AM) objected:
>> The problem with that is that I was given to understand that
>> duplicated keys is actually malformed JSON - perhaps I misunderstood.
>> If that's legitimate JSON, then I'm fine with just one.
and, overlapping with my follow-up correcting that,
Kai Köhne (15 February 2023 17:10) replied:
> To my understanding it's valid JSON, at least from the syntax
> side. From
> https://www.ecma-international.org/publications-and-standards/standards/ecma-404/
> :
> The JSON syntax does not impose any restrictions on the strings used
> as names, *does not require that name strings be unique*, and does
> not assign any significance to the ordering of name/value
> pairs. These are all semantic considerations that may be defined by
> JSON processors or in specifications defining specific uses of JSON
> for data interchange
It seems we read the same standard at about the same time, arriving at
the same conclusion ;^>
> And the JSON parsers I tested (Python, Qt) don't treat it as an error,
> either. There seems to be some online linters like
> https://jsonlint.com/ that complain about it, tough.
I think we can live with ignoring a warning from linters that aren't
part of our tool-chain ;^>
I've updated my reviews to the "Comment" version.
That just leaves us with the open question of whether to, instead,
switch to some other way of storing the data - preferably one that
supports multi-line strings - with Ulf currently championing QML and
Thiago YAML.
Eddy.
--
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development
Re: [Development] Support for *Notes and UpstreamFiles fields in qt_attributions.json files
QML supports comments, but not multi-line string literals. You can concatenate them, though. Sure it does. You can use ECMAScript template strings: property string longthing: `a multi line string` (In fact you can also just sprinkle line breaks into your regular strings. But don't.) -- Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
Re: [Development] Support for *Notes and UpstreamFiles fields in qt_attributions.json files
On Wednesday, 15 February 2023 02:15:02 PST Ulf Hermann via Development wrote: > Or QML, in fact: QML supports comments, but not multi-line string literals. You can concatenate them, though. > That would be all nicely human-readable and easily verified with > qmllint. Which you also get with YAML. And since YAML is just a "front-end" to JSON in most cases, you can use all the JSON tools to query and manipulate the contents, like jq and jp. -- Thiago Macieira - thiago.macieira (AT) intel.com Cloud Software Architect - Intel DCAI Cloud Engineering smime.p7s Description: S/MIME cryptographic signature -- Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
Re: [Development] Support for *Notes and UpstreamFiles fields in qt_attributions.json files
On Tuesday, 14 February 2023 23:35:57 PST Kai Köhne via Development wrote: > I'd be open to all kinds of formats if we'd start on a green field. But > we're not, and I don't think reimplementing qtattributionsscanner + redo > the build system integration + convert all existing > qt_attributionsscanner.json files to a new format is worth the time. > Unless, of course, somebody volunteers I can volunteer to patch qtattributionsscanner to run a small Python script to convert from YAML to JSON before the parsing. -- Thiago Macieira - thiago.macieira (AT) intel.com Cloud Software Architect - Intel DCAI Cloud Engineering smime.p7s Description: S/MIME cryptographic signature -- Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
Re: [Development] Support for *Notes and UpstreamFiles fields in qt_attributions.json files
> -Original Message-
> From: Edward Welbourne
> Sent: Wednesday, February 15, 2023 10:45 AM
> To: Kai Köhne
> Cc: Development@qt-project.org
> Subject: Re: Support for *Notes and UpstreamFiles fields in
> qt_attributions.json
> files
>
> Kai Köhne (15 February 2023 08:50) replied:
> > did you intentionally sent this off-list?
>
> oops - no, outlook's UI tricked me :-(
> And, in fact, it just did it again, although I'm sure I hit Reply All this
> time.
> Manually adding development to CC...
>
> For the benefit of everyone else, my reply is below, after the bit of Kai's
> earlier
> mail, that I quoted:
>
> >>> Anyhow, I wonder whether it wouldn't suffice to have _one_ comments
> >>> field, instead of a dedicated UpstreamFiles field, and *Notes fields
> >>> for every single entry. E.g.
> >>>
> >>> {
> >>> "Comments": [
> >>> "Upstream files were copied from:",
> >>> " src/dir1, src/dir2",
> >>> "The license and copyright was derived from dist/LICENSE.txt"
> >>> ]
> >>> }
> >>>
> >>> The benefit I see is that qtattributionsscanner (and any other JSON
> >>> tool that might be used by others) has only to care about one
> >>> additional field, not multiple ones.
>
> Edward Welbourne (Tuesday, February 14, 2023 7:37 PM) had written:
> >> I see the case for it, but it comes at the cost of all the comments
> >> being in one place, not each comment next to the part of the content it
> relates to.
> >>
> >> If someone is updating one of many data in an attribution and the
> >> comments aren't close at hand, both the author of the change and the
> >> reviewers may fail to notice the detail that the comment mentions
> >> that makes it obvious they're doing something wrong.
> >>
> >> That's not a fatal argument: each attribution is fairly short, so
> >> putting the comments in the middle should make it easy to see them
> >> when looking at any of the lines they relate to. None the less,
> >> comments and documentation belong close to the code they relate to ...
>
> > Well, you can also achieve this by duplicating comment fields:
> >
> > {
> > "Comment": "Upstream files are src/x.cpp, include/y.h",
> > "Files": [ "x.cpp", "y_p.h"]
> > "Comment": "Copyright info is from dist/COPYING",
> > "Copyright": "Copyright (C) 2023 Joe Doe"
> > }
>
> The problem with that is that I was given to understand that duplicated keys
> is
> actually malformed JSON - perhaps I misunderstood. If that's legitimate JSON,
> then I'm fine with just one.
To my understanding it's valid JSON, at least from the syntax side. From
https://www.ecma-international.org/publications-and-standards/standards/ecma-404/
:
The JSON syntax does not impose any restrictions on
the strings used as names, *does not require that name strings be unique*,
and does not assign any
significance to the ordering of name/value pairs. These are all semantic
considerations that may be defined by
JSON processors or in specifications defining specific uses of JSON for data
interchange
And the JSON parsers I tested (Python, Qt) don't treat it as an error, either.
There seems to be some online linters like https://jsonlint.com/ that complain
about it, tough.
> > I just don't see the point of dedicated fields if the whole purpose is
> > to ignore them in the tooling.
>
> Well, there's more tooling to consider - someone, at least, has run a JSON-
> validation checker on some qt_attributions.json files and submitted patches to
> fix issues reported there (like line-breaks instead of \n; and I think that's
> where
> I heard about duplicate keys being invalid) - and in any case there are
> developers who need to read it, who may benefit from the keys having names
> that makes clear which tooling-relevant keys they relate to.
Right. But this can also work the other way: If we ever come around to
formalizing a json-schema, then having to list all the *Notes fields
complicates things, too.
I'm still for the KISS principle and go for one Comment field. If we have a
need for more 'local' fields in some cases, let's just allow duplicating these.
We can always reconsider this in the future.
Kai
--
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development
Re: [Development] Support for *Notes and UpstreamFiles fields in qt_attributions.json files
So I finally found time to look at the JSON specification [0] and find that (in section 6) it says, of the name/value pairs in an object: The JSON syntax does not impose any restrictions on the strings used as names, does not require that name strings be unique, [0] PDF linked from ECMA 404, https://www.ecma-international.org/publications-and-standards/standards/ecma-404/ It's possible that some JSON linters or tools complain about non-unique keys, but they're not an *abuse* as such, so we can legitimately go with the simpler approach Kai has proposed, just one field, Comment (taking any data as value) will do. I'll amend my proposed change to QUIP 7 accordingly, Eddy. -- Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
Re: [Development] Support for *Notes and UpstreamFiles fields in qt_attributions.json files
Or QML, in fact:
// Attribution.qml in QtAttribution module:
QtObject {
property string name
property list files
property list upstreamFiles
//
}
// Actual attribution.qml in source tree:
import QtAttribution
Attribution {
// allows comments as much as you like
name: "Foobar"
files: ["some/file.h", "some/file.cpp"]
license: License.GPL // can be a QML enum in a License.qml
// ...
}
That would be all nicely human-readable and easily verified with
qmllint. I get the argument for sticking with JSON now, though.
best,
Ulf
--
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development
Re: [Development] Support for *Notes and UpstreamFiles fields in qt_attributions.json files
Kai Köhne (15 February 2023 08:50) replied:
> did you intentionally sent this off-list?
oops - no, outlook's UI tricked me :-(
And, in fact, it just did it again, although I'm sure I hit Reply All this time.
Manually adding development to CC...
For the benefit of everyone else, my reply is below, after the bit of
Kai's earlier mail, that I quoted:
>>> Anyhow, I wonder whether it wouldn't suffice to have _one_ comments
>>> field, instead of a dedicated UpstreamFiles field, and *Notes fields
>>> for every single entry. E.g.
>>>
>>> {
>>> "Comments": [
>>> "Upstream files were copied from:",
>>> " src/dir1, src/dir2",
>>> "The license and copyright was derived from dist/LICENSE.txt"
>>> ]
>>> }
>>>
>>> The benefit I see is that qtattributionsscanner (and any other JSON
>>> tool that might be used by others) has only to care about one
>>> additional field, not multiple ones.
Edward Welbourne (Tuesday, February 14, 2023 7:37 PM) had written:
>> I see the case for it, but it comes at the cost of all the comments being in
>> one
>> place, not each comment next to the part of the content it relates to.
>>
>> If someone is updating one of many data in an attribution and the comments
>> aren't close at hand, both the author of the change and the reviewers may
>> fail
>> to notice the detail that the comment mentions that makes it obvious they're
>> doing something wrong.
>>
>> That's not a fatal argument: each attribution is fairly short, so putting the
>> comments in the middle should make it easy to see them when looking at any
>> of the lines they relate to. None the less, comments and documentation
>> belong
>> close to the code they relate to ...
> Well, you can also achieve this by duplicating comment fields:
>
> {
> "Comment": "Upstream files are src/x.cpp, include/y.h",
> "Files": [ "x.cpp", "y_p.h"]
> "Comment": "Copyright info is from dist/COPYING",
> "Copyright": "Copyright (C) 2023 Joe Doe"
> }
The problem with that is that I was given to understand that duplicated
keys is actually malformed JSON - perhaps I misunderstood. If that's
legitimate JSON, then I'm fine with just one.
> I just don't see the point of dedicated fields if the whole purpose is
> to ignore them in the tooling.
Well, there's more tooling to consider - someone, at least, has run a
JSON-validation checker on some qt_attributions.json files and submitted
patches to fix issues reported there (like line-breaks instead of \n;
and I think that's where I heard about duplicate keys being invalid) -
and in any case there are developers who need to read it, who may
benefit from the keys having names that makes clear which
tooling-relevant keys they relate to.
Eddy.
--
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development
Re: [Development] Support for *Notes and UpstreamFiles fields in qt_attributions.json files
> -Original Message- > From: Development On Behalf Of Ulf > Hermann via Development > Subject: Re: [Development] Support for *Notes and UpstreamFiles fields in > qt_attributions.json files > > YAML is really quite terrible. If we're going to switch, let's choose > something > else. > > Basically, YAML is extremely complex, ambiguous, and incompatible between > different versions. See for example > https://ruudvanasseldonk.com/2023/01/11/the-yaml-document-from-hell for > an in-depth explanation. > > Toml seems to be popular these days. I'd be open to all kinds of formats if we'd start on a green field. But we're not, and I don't think reimplementing qtattributionsscanner + redo the build system integration + convert all existing qt_attributionsscanner.json files to a new format is worth the time. Unless, of course, somebody volunteers Regards Kai -- Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
Re: [Development] Support for *Notes and UpstreamFiles fields in qt_attributions.json files
YAML is really quite terrible. If we're going to switch, let's choose something else. Basically, YAML is extremely complex, ambiguous, and incompatible between different versions. See for example https://ruudvanasseldonk.com/2023/01/11/the-yaml-document-from-hell for an in-depth explanation. Toml seems to be popular these days. best regards, Ulf -- Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
Re: [Development] Support for *Notes and UpstreamFiles fields in qt_attributions.json files
On Tuesday, 14 February 2023 07:07:00 PST Kai Köhne via Development wrote: > First, let's agree that JSON sucks for the task at hand. It doesn't have any > explicit support for comments, and no support for multi-line strings > (though our implementation tolerates this). How about switching to YAML? Those files are consumed by qtattributionsscanner. That looks like a very simple application that can be rewritten in Python, or it could launch Python or yq to convert YAML to JSON on the fly. -- Thiago Macieira - thiago.macieira (AT) intel.com Cloud Software Architect - Intel DCAI Cloud Engineering smime.p7s Description: S/MIME cryptographic signature -- Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
Re: [Development] Support for *Notes and UpstreamFiles fields in qt_attributions.json files
Hi,
+1 to the approach suggested by Kai.
Having comments would be very helpful, but I do not think that we need a
separate comment field for each entry.
Best regards,
Ivan
From: Development on behalf of Kai Köhne
via Development
Sent: Tuesday, February 14, 2023 4:07 PM
To: Edward Welbourne ; Development@qt-project.org
Subject: Re: [Development] Support for *Notes and UpstreamFiles fields in
qt_attributions.json files
Hi Eddy,
> -Original Message-
> From: Development On Behalf Of
> Edward Welbourne via Development
> Sent: Tuesday, February 14, 2023 3:14 PM
> To: Development@qt-project.org
> Subject: [Development] Support for *Notes and UpstreamFiles fields in
> qt_attributions.json files
>
> Hi all,
>
> Having taken part in various third-party updates and felt a need to leave
> notes
> for those who will do the same in future, I have run up against JSON not
> having
> a comment format. To work round that, I propose to allow some fields to be
> included in a qt_attribution.json file for that purpose. As a general
> pattern, I
> propose allowing ${Field}Notes for any ${FIELD} that already exists; and a
> separate UpstreamFiles field, corresponding to the Files one, to list where in
> the upstream source tree to find the files that are to be copied.
First, let's agree that JSON sucks for the task at hand. It doesn't have any
explicit support for comments, and no support for multi-line strings (though
our implementation tolerates this).
Anyhow, I wonder whether it wouldn't suffice to have _one_ comments field,
instead of a dedicated UpstreamFiles field, and *Notes fields for every single
entry. E.g.
{
"Comments": [
"Upstream files were copied from:",
" src/dir1, src/dir2",
"The license and copyright was derived from dist/LICENSE.txt"
]
}
The benefit I see is that qtattributionsscanner (and any other JSON tool that
might be used by others) has only to care about one additional field, not
multiple ones.
Regards
Kai
--
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development
--
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development
Re: [Development] Support for *Notes and UpstreamFiles fields in qt_attributions.json files
Hi Eddy,
> -Original Message-
> From: Development On Behalf Of
> Edward Welbourne via Development
> Sent: Tuesday, February 14, 2023 3:14 PM
> To: Development@qt-project.org
> Subject: [Development] Support for *Notes and UpstreamFiles fields in
> qt_attributions.json files
>
> Hi all,
>
> Having taken part in various third-party updates and felt a need to leave
> notes
> for those who will do the same in future, I have run up against JSON not
> having
> a comment format. To work round that, I propose to allow some fields to be
> included in a qt_attribution.json file for that purpose. As a general
> pattern, I
> propose allowing ${Field}Notes for any ${FIELD} that already exists; and a
> separate UpstreamFiles field, corresponding to the Files one, to list where in
> the upstream source tree to find the files that are to be copied.
First, let's agree that JSON sucks for the task at hand. It doesn't have any
explicit support for comments, and no support for multi-line strings (though
our implementation tolerates this).
Anyhow, I wonder whether it wouldn't suffice to have _one_ comments field,
instead of a dedicated UpstreamFiles field, and *Notes fields for every single
entry. E.g.
{
"Comments": [
"Upstream files were copied from:",
" src/dir1, src/dir2",
"The license and copyright was derived from dist/LICENSE.txt"
]
}
The benefit I see is that qtattributionsscanner (and any other JSON tool that
might be used by others) has only to care about one additional field, not
multiple ones.
Regards
Kai
--
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development
[Development] Support for *Notes and UpstreamFiles fields in qt_attributions.json files
Hi all,
Having taken part in various third-party updates and felt a need to
leave notes for those who will do the same in future, I have run up
against JSON not having a comment format. To work round that, I propose
to allow some fields to be included in a qt_attribution.json file for
that purpose. As a general pattern, I propose allowing ${Field}Notes
for any ${FIELD} that already exists; and a separate UpstreamFiles
field, corresponding to the Files one, to list where in the upstream
source tree to find the files that are to be copied.
This will make the work easier for those who do third-party updates and
avoid various kludges presently in use to work round the lack of
comments (for example, abusing an existing field to first write the note
and then over-write it with the value; the tools that process the files
cope, but it's malformed JSON).
See:
* https://codereview.qt-project.org/c/meta/quips/+/460358
* https://codereview.qt-project.org/c/qt/qttools/+/460116
* https://codereview.qt-project.org/c/qt/qttools/+/460181
* https://codereview.qt-project.org/c/qt/qtbase/+/458824
The last triggered this; the previous two implement it and the first is
an update to QUIP 7.
Eddy.
--
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development
