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

Create new att.source and include att.written and att.placement in it #2643

Closed
wants to merge 3 commits into from

Conversation

sabineseifert
Copy link
Contributor

and also added note to description of att.placement concerning its use.

An 'good' example gently guiding the use of place in e.g. div is still missing.

and also added note to description of att.placement concerning its use
@sabineseifert sabineseifert linked an issue Dec 23, 2024 that may be closed by this pull request
@sabineseifert sabineseifert added this to the Guidelines 4.9.0 milestone Dec 23, 2024
Copy link
Member

@ebeshero ebeshero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sabineseifert About this, I was just re-reading our minutes from Buenos Aires. It seems we were going to move the classes att.written and att.placement into a new class called att.source. And we left some guidance about parallel cases to help with this. Take a look (you can find the discussion in the F2F minutes by searching for @place on the posted minutes on the website.)

@sabineseifert sabineseifert removed a link to an issue Dec 30, 2024
@sabineseifert sabineseifert linked an issue Dec 30, 2024 that may be closed by this pull request
Also added att.written and att.placement to new att.source
@sabineseifert
Copy link
Contributor Author

sabineseifert commented Dec 30, 2024

This PR was tied to #2551 but is now tied to #2550.

What I did:

  • created new class att.source -- Please review carefully what might be missing/wrong here. (E.g. is there an xml:id needed in <classSpec>, like xml:id="class-attr-source"?)
  • added att.written and att.placement to new att.source (both as discussed at F2F in October 2024)
  • added a remark to att.placement as discussed in Council meeting 2024-09-03 (in relation to Allow @place in <div>  #2551)

What I did not:

@sabineseifert sabineseifert changed the title Added att.placement to att.global.rendition Create new att.source and include att.written and att.placement in it Dec 30, 2024
This was referenced Dec 30, 2024
@sydb
Copy link
Member

sydb commented Dec 30, 2024

@sabineseifert — I presume this should currently be considered a DRAFT pull request, yes?

Although you have created att.source, you have not added it to the Guidelines. It needs to be included in the list at #DSTECAT in Source/Guidelines/en/ST-Infrastructure.xml. (Starts on line ~864.) Adding it is trivially easy. Adding it in the right place can sometimes be tricky.

As for @xml:id on <classSpec>, we use it about ½ the time. Will look in a moment at how often we reference it, but I suspect it is very rarely.

Could someone remind me why we are calling this new class “att.source” — seems confusingly close to att.global.source. Maybe att.renditionLike, att.styleLike, att.sourceDetails, att.specificRendition, or some such?

@sydb
Copy link
Member

sydb commented Dec 30, 2024

Of the just over 100 @xml:id attributes we have on the over 200 <classSpec>s in the Guidelines source, exactly 0 of them are referenced.

@sabineseifert sabineseifert marked this pull request as draft December 31, 2024 10:54
@sabineseifert
Copy link
Contributor Author

Good idea to mark this as a draft PR which I just did!

I looked at the ST-Infrastructure.xml and must confess that I wouldn't know where to add it. But before, we need to settle the question of the name anyway: att.source.rendition and att.source were proposed at the F2F in October 2024). And: the parallel case model.phrase and model.limitedPhrase might help with naming and defining the new class. But I think we did not finish discussing the name.

And concerning the 0 references to the over 100 @xml:ids: We probably should not keep on going adding these but we perhaps should not delete the already existing ones?

Thanks, @sydb, for looking into this!

@sydb
Copy link
Member

sydb commented Dec 31, 2024

I am ducking the name issue for the moment, mostly because I do not really know what to call this thing.

But as for where in ST the <xi:include href="../../Specs/att.WHATEVER.xml"/> goes, it should be inserted somewhere between lines 864 and 929. As for where within that list, the only requirement (IIRC) is that an attribute class which has a <memberOf key="DUCK"> has to be after the <xi:include href="../../Specs/att.DUCK.xml"/>. Thus those includes that refer to classes do NOT have a <memberOf> can go anywhere, and we generally list them alphabetically.

About ½ of our attribute classes are a member of some other class. But of course, an average by accident roughly ½ of those should be in the correct order without intervention. I am working on re-organizing that list and commenting it better, but will not get to check anything in for hours, I am sure.

As for adding @xml:id to <classSpec> — I do not think it matters.

@sydb
Copy link
Member

sydb commented Jan 21, 2025

I am going to propose a different solution.

I note that the majority of elements that are in att.placement are already in att.written. Thus I am wondering (aloud, first to @ebeshero then here) if we should perhaps just add att.placement to att.written. The result would be only 4 elements that currently do not have @hand gaining it: <metaMark>, <notatedMusic>, <rt>, and <witDetail>.

This might not work, because some elements that are in att.placement are already in att.placement, so we might have some conflict problems. But it is worth investigating.

Here are the elements that are either directly or indirectly in att.placement and att.written:

placement written
ab
add add
addSpan addSpan
closer
damage
damageSpan
dateline
del
delSpan
div
emph
figure figure
fw fw
head head
hi
label label
metamark
lem
line
mod
notatedMusic
note note
noteGrp noteGrp
rt
opener
p
path
postscript
rdg
rdgGrp
redo
restore
retrace
rt
salute
seg
signed
stage stage
subst
substJoin
text
trailer trailer
undo
witDetail
zone

@sydb
Copy link
Member

sydb commented Jan 21, 2025

As sort of expected, the error “error: duplicate attribute "hand"” occurs almost immediately on trying the above proposal. So the question becomes, how much class-jiggering would be needed to avoid that problem, and is it worth it?

@ebeshero
Copy link
Member

ebeshero commented Jan 21, 2025

@sydb I wonder if the common ancestor idea is still a good one: att.written is a member of att.transcriptional. What about also including att.placement in att.transcriptional? Doesn't that effectively do what we initially proposed in Buenos Aires, and prevent some of the conflict, too? It also seems like the appropriate class--the description seems more applicable to att.placement than the description of att.written.

https://tei-c.org/release/doc/tei-p5-doc/en/html/ref-att.transcriptional.html

@martindholmes
Copy link
Contributor

I really don't like the name att.source. It's going to be completely confusing; most of these attributes have nothing to do with the source of anything, except that they're used in transcription, and transcription is usually of some kind of source.

@ebeshero ebeshero removed this from the Guidelines 4.9.0 milestone Jan 22, 2025
@ebeshero
Copy link
Member

ebeshero commented Jan 22, 2025

Work on the issues to be covered by this PR was moved to another PR: #2665
This branch wasn't passing tests and we started down a different path for a solution!

@ebeshero ebeshero closed this Jan 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow @hand in <emph>; Allow @hand in <dateline>
4 participants