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

Filling out the table contents in the document tree. #5

Merged
merged 3 commits into from
May 9, 2019

Conversation

andreubotella
Copy link

I'm filling out the table content model according to the declarations in soexblx.dtd. So far I've added the elements, next I'll be adding the attributes.

@flying-sheep
Copy link
Owner

great, thank you! I know there’s still a lot to do here, so thank you for moving it along!

@andreubotella
Copy link
Author

andreubotella commented May 5, 2019

After taking a good look at the attributes of the table elements in soextblx.dtd and comparing with the ReST spec, it looks like the only attributes that are needed in a straightforward implementation of the spec are morerows and morecols (the latter defined in the tbl.entry.att entity in docutils.dtd) as attributes of entry, which act like rowspan and colspan minus one. And if we're going by what the spec needs as opposed to what Docutils uses, the tgroup and colspec elements wouldn't be needed either.

But though the spec doesn't require the attributes and elements I listed above, docutils does use the colwidth attribute to colspec to set a CSS width for the columns depending on how wide they are in the source. I don't think that's part of the ReST spec, or whether this implementation might want to follow that.

Should I keep everything that's part of the Docutils doctype, or get rid of anything not needed?

@flying-sheep
Copy link
Owner

Up until now I followed everything the DTD does, as I’d like to (de)serialize the doctrees to docutils XML. On the other hand, if some attributes are flat out unnecessary, we could just ignore them when deserializing.

Do what you think is best!

@andreubotella
Copy link
Author

In the end I've implemented every attribute, since if you're contemplating serializing into XML as a possible feature, parsing a docutils XML isn't that far-fetched.

I got a type error about the cols attribute in tgroup: since it's a required attribute of number type, I implemented it with type usize (rather than with Option as I did on implied attributes), but apparently attribute types must implement CanBeEmpty. Should I create a tuple struct that simply wraps the usize and implements that trait?

@flying-sheep
Copy link
Owner

Sounds ideal! Thank you.

@andreubotella
Copy link
Author

tgroup is the first element implemented so far that has both children and a required attribute, and the macro impl_children! doesn't work with structs that don't implement Default (and more generally, it's impossible to implement HasChildren::with_children for a type with non-default extra attributes). So I worked around that restriction by having attribute_types::TableGroupCols default to 0, although that doesn't reflect the DTD. If you see a better way to solve this, let me know.

@flying-sheep
Copy link
Owner

Damn, that’s a design issue for sure. I think your comment there is sufficient. Thank you!

@flying-sheep flying-sheep merged commit ecb69f0 into flying-sheep:master May 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants