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

termid is not always integer #1

Closed
strogonoff opened this issue Jan 13, 2019 · 7 comments
Closed

termid is not always integer #1

strogonoff opened this issue Jan 13, 2019 · 7 comments

Comments

@strogonoff
Copy link

strogonoff commented Jan 13, 2019

In the resulting YAML, termid is sometimes a string, sometimes an integer:

foo@bar:geolexica.com$ find . -name '*.adoc' | xargs fgrep termid
./_concepts/894.adoc:termid: 894
./_concepts/1458.adoc:termid: 1458
./_concepts/206.adoc:termid: '206'
./_concepts/1233.adoc:termid: 1233

Generally an unwelcome inconsistency, it also prevents sorting concepts by term ID in restricted environments such as Liquid templates.

I think a conversion can be added here (not sure whether this is the correct spot and what is the best way to do this in Ruby):

https://github.com/riboseinc/tc211-termbase/blob/46de3c8992fd4efa952e77e235aaedeb772ec627/lib/tc211/termbase/concept.rb#L35

@ronaldtse
Copy link
Member

@strogonoff I agree, and that should be the correct place. Let's make them integers then? I'm actually not sure if Jekyll can do Integer sorting...?

@strogonoff
Copy link
Author

Liquid allows sorting by a key and if it’s an integer then I assume it’ll sort by the value. I’m +1 on casting each string to integer unless we actually expect non-integer term IDs.

Something like "termid" => id.to_i on the above-mentioned line might work?

@strogonoff
Copy link
Author

perhaps "termid" => Integer(id) is better since that’d ensure it fails on bad input

@ronaldtse
Copy link
Member

Perhaps -- I kind of remember that Liquid cannot sort Integers. I think that's why I didn't make the sort happen. Can you double check?

@strogonoff
Copy link
Author

@ronaldtse I see what you mean, there are reports that everything in Jekyll document frontmatter gets cast to strings. (Although if everything gets cast to string, it’s super confusing why I was getting what seemed like type mismatch error when trying to sort by termid specifically.)

In any case it’s a generic library without any promise of Jekyll support specifically so IMO it’s not Jekyll sites that should be the main motivation to fix this, just general consistency of output. It might or might not make Geolexica work, I wouldn’t put extra priority because of that.

@ronaldtse
Copy link
Member

Correct, this is a generic library. If possible, feel free to help fix this 👍

@ronaldtse
Copy link
Member

Fixed, but need to add specs. (in #8)

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

No branches or pull requests

2 participants