Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unnecessary comments from ODD customization exemplars #1987

Closed
raffazizzi opened this issue Apr 15, 2020 · 9 comments
Closed

Remove unnecessary comments from ODD customization exemplars #1987

raffazizzi opened this issue Apr 15, 2020 · 9 comments
Assignees

Comments

@raffazizzi
Copy link
Contributor

raffazizzi commented Apr 15, 2020

For example https://github.com/TEIC/TEI/blob/dev/P5/Exemplars/tei_all.odd has <!-- required --> attributes next to the main modules in the TEI. However, one can still choose to get rid of those and using Roma (beta), those pesky comments linger around (see TEIC/romajs#95).

Why don't we get rid of them and perhaps similar ones in other exemplar customization ODDs? Don't think they're very helpful.

@martindholmes
Copy link
Contributor

Those modules are required AFAIK if you're intending to generate a customization that's TEI-compliant. It took me a little while to remember those four, and I remember starting my own customizations from tei_all and taking note of those comments, so I have actually found them useful.

@lb42
Copy link
Member

lb42 commented Apr 15, 2020

Strictly speaking, it's not the modules which are required for conformance but certain elements (e.g. teiHeader, fileDesc) they provide. How about providing more details for these comments (e.g. "This module is required because it provides elements teiHeader and fileDesc and several others required for TEI Conformance") . As they stand I agree they are a bit mystifying.

@martindholmes
Copy link
Contributor

@lb42 good point. I do like the idea of improving the comments rather than deleting them. So many people start their projects with tei_all, and when they eventually get around to trimming their schema, if they do it through an ODD file (which I hope many do), this kind of documentation will be useful.

@sydb
Copy link
Member

sydb commented Apr 15, 2020

When I put those comments in, those modules were actually required, because it was before we had @include and @except, let alone <elementRef>. Nowadays, as @lb42 points out, they are not actually required, only certain elements are. But I do think they are (mildly) useful, so I like Lou’s idea of expanding them.
Perhaps “includes required elements <teiHeader>, <fileDesc>, …” or some such.

@jamescummings
Copy link
Member

'required' means a lot of different things to other people. It just seems weird to me to have a moduleRef like:

<moduleRef key="header"
          except="appInfo application biblFull cRefPattern calendar calendarDesc            
          catRef classCode conversion creation refState refsDecl samplingDecl            
          scriptNote stdVals styleDefDecl tagUsage tagsDecl unitDecl unitDef xenoData"/>!
<!-- required -->
<moduleRef key="linking" except="alt altGrp join joinGrp timeline when"/>

interspersed amongst other modules. Indeed, does the comment apply to the header or linking? (Obviously, we know which.... but that highlights it is not clear.)

I would be more useful to have a single comment at the beginning of the schemaSpec saying something like:

<!-- It is probably a good idea to have the 'tei' module and some elements 
     from the 'core', 'header', and 'textstructure' modules unless you really 
     know what you are doing. -->

@sydb
Copy link
Member

sydb commented Sep 5, 2020

On first thought I very much like the idea of improving the comments. But no matter how good they are, as comments they still present @raffazizzi (well, Romaβ) with a problem: if someone starts with the customization exemplar and removes a “required” module, the comment remains and can cause confusion.

I am thus favoring a chunk of prose (along the lines of @jamescummings’ suggestion, above) as part of the content of the ODD file, probably in a <p> just before the <schemaSpec>.

I also wonder if there shouldn’t be an exemplar customization that has something like:

      <specGrp xml:id="required_modules">
        <p>This specification groups together the <soCalled>required</soCalled>
        modules. These modules are not themselves actaually required to create
        a TEI conformant markup language (let alone a useful non-TEI conformant
        markup language). Rather, they each contain one or more <emph>elements</emph>
        which are required for TEI conformance. For a list of required elements
        and the modules from which each comes, see <ptr target="#rel"/>.</p>
        <moduleRef key="tei"/>
        <moduleRef key="textstructure"/>
        <moduleRef key="core"/>
        <moduleRef key="header"/>
      </specGrp>
      <!-- ... -->
      <schemaSpec ident="myTEI">
        <specGrpRef target="#required_modules"/>
        <!-- ... -->
      </schemaSpec>

@martinascholger
Copy link
Member

VF2V agrees with @sydb's proposal to turn comments into a proper documentation.

@sydb
Copy link
Member

sydb commented Oct 26, 2020

Only 13 exemplars have the word “required” in a comment:

Exemplars/tei_all.odd:4
Exemplars/tei_allPlus.odd:4
Exemplars/tei_corpus.odd:1
Exemplars/tei_customization.odd:3
Exemplars/tei_docs.odd:4
Exemplars/tei_drama.odd:1
Exemplars/tei_enrich.odd:1
Exemplars/tei_minimal.odd:4
Exemplars/tei_ms.odd:1
Exemplars/tei_simplePrint.odd:1
Exemplars/tei_speech.odd:1
Exemplars/tei_svg.odd:1
Exemplars/tei_tite.odd:2

Of those, tei_customization is generated, the comment in tei_simplePrint is not one of the ones @raffazizzi is complaining about, and the ones in tei_tite may or may not be problematic (but are different in nature, anyway). Thus I am not planning to tackle those exmplars. And the ones in tei_minimal are much more explanatory, and it is @jamescummings’ puppy anyway, so I’ll leave that one for him. That leaves:

  • tei_all.odd
  • tei_allPlus.odd
  • tei_corpus.odd
  • tei_docs.odd
  • tei_drama.odd
  • tei_ms.odd
  • tei_speech.odd
  • tei_svg.odd

I plan to tick them off here as I edit them and commit them to my local dev branch, then push them all at once (once local tests are passing).

sydb added a commit that referenced this issue Oct 27, 2020
@sydb
Copy link
Member

sydb commented Oct 27, 2020

For tei_all and tei_allPlus I updated the prose somewhat (primarily to replace the comments), sorted the <moduleRef>s into definition order, and added @n attributes to each, in addition to nuking the offending comments.

For tei_allPlus I also removed the extraneous definitions for <egXML> and %macro.anyThing;.

For all the others I may have tweaked the prose a bit, but for the most part just removed the offending comments.

And I have to say, it is an odd feeling trying to make microscopic improvements to ODD files that I sincerely believe should not exist (tei_corpus, tei_docs, tei_ms, tei_speech; and, to a lesser extent, tei_drama and tei_svg).

@sydb sydb closed this as completed Oct 27, 2020
@martinascholger martinascholger added this to the Guidelines 4.2.0 milestone Feb 8, 2021
hcayless pushed a commit that referenced this issue Jun 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants