usnistgov / OSCAL

Open Security Controls Assessment Language (OSCAL)
https://pages.nist.gov/OSCAL/
Other
667 stars 181 forks source link

Nonuniform indentation in metaschemas #1183

Open guyzyl opened 2 years ago

guyzyl commented 2 years ago

I recently noticed that there isn't an indentation convention/uniformity amongst the metaschema files in src/metaschema. The indentation sizes that exist (all in # of spaces) are:

This doesn't hurt functionality at all, just isn't very conventional for a code repo to have different indentations for same file types.

Are you open to a PR fixing all indentations to a single convention? My preference is towards 4 spaces (let me know if you think otherwise).


Side note - it might a good idea to consider using an XML linting utility during the CI/CD process, but that's the topic for another issue.

wendellpiez commented 2 years ago

(Discussed with @david-waltermire-nist and @aj-stein-nist March 22.) A one-time fix will unfortunately not defend against slippage.

There is a variety of different content formats in the OSCAL repo, including JSON, YAML, Markdown and XML. We need to normalize whitespace styles across repositories, for reasons including supporting useful differencing. For a given contribution, we need a way to ensure that the contribution conforms to (house) whitespace rules.

Also, we want contributors to be able to edit repo content using whatever whitespace style they like best, avoiding imposing whitespace normalization rules during development. Instead, we would like the rules to be enforced only when the PR is made.

The repository CI/CD needs a way to check and enforce these normalization rules. Also when making a PR, devs need tooling to reformat whitespace easily, based on the imposed normalization requirements.

To support this, we need tools that can perform both the normalization and the rules enforcement. We would like input from the community on how to address this in JSON, YAML, XML, and Markdown.

Any suggestions on how this is done?

david-waltermire commented 2 years ago

Raised this on the 3/24/2022 Lunch with the Devs call. We didn't get any feedback for or against working on whitespace normalization. I asked the community to provide feedback on this issue after the meeting.

guyzyl commented 2 years ago

@david-waltermire-nist unfortunately I wasn't able to make it to today's lunch with devs and voice my opinion, but it is the one presented in this issue (surprisingly as I'm the one who created it 😅).

GaryGapinski commented 2 years ago

Just an FYI.

As an experiment, I created a large (100Mb) XML document with no line feeds or indentation and used the following transform to serialize it with Saxon HE 11.1. The output was passable but not quite what I would have expected (and not the style I prefer).

I will also note that https://www.w3.org/TR/xml-c14n11/ is unlikely to be acceptable (as a means by which routine code maintenance and git diffs are tidy).

<?xml version="1.0" encoding="UTF-8"?>
<xsl:stylesheet xmlns:xsl="http://www.w3.org/1999/XSL/Transform"
    xmlns:xs="http://www.w3.org/2001/XMLSchema"
    xmlns:math="http://www.w3.org/2005/xpath-functions/math"
    exclude-result-prefixes="xs math"
    version="3.0">
    <xsl:output method="xml" indent="true"/>
    <xsl:mode on-no-match="deep-copy"/>
</xsl:stylesheet>
Arminta-Jenkins-NIST commented 11 months ago

At the 11/16 Triage Meeting: This issue is insignificant functionally but GitHub can misinterpret the white space. NIST team agreed to defer to @wendellpiez and discuss again on Nov 30th.

wendellpiez commented 11 months ago

Is the problem noise in diffing, or (just) unsightly display when looking at the metaschema source?

In any case there are a couple of mitigations possible, with the easiest very much depending on what XML-wrangling tools you are comfortable with. I'm happy to discuss approaches here.

The comprehensive approach hinted at in the discussion thread is probably out of scope for now. Although we can discuss what it would be. (But it is not an OSCAL solution, it is an all-Metaschema solution that would require OSCAL implementation. I suggest an OSCAL solution.)

For the next person touching the metaschemas to do some whitespace fixup -- this is definitely doable, particularly using the right tool.

@Arminta-Jenkins-NIST one possibility is to bundle this Issue with the next Issue that touches Metaschema source (maybe @Compton-NIST can help determine what that is?), and I can provide guidance and assistance with the task.