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

[Кино] add Группа крови w/ translation mapping #724

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

snshn
Copy link
Member

@snshn snshn commented Feb 5, 2022

Would like to add this in order for lyrics-website-generator's translation mapping parser to have something to work with in the production database.

A couple of questions:

  • what do you think of the file extension, do you like it?
  • I took the format from our discussion thread, did I take the right snippet?

@snshn snshn requested a review from defanor February 5, 2022 05:45
Copy link

@defanor defanor left a comment

Choose a reason for hiding this comment

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

what do you think of the file extension, do you like it?

I'm not picky about filename extensions, and actually not quite sure if it needs one: since the corresponding files in other directories (original texts, translations) don't have them, maybe it'd be more consistent (and possibly less confusing to deal with them later, from the generator) to not have them here either. But not a big deal either way.

I took the format from our discussion thread, did I take the right snippet?

Yup.

@snshn
Copy link
Member Author

snshn commented Feb 5, 2022

I can see translations existing without maps, but never maps without translations. There's already two (actually, 3) very deep filesystem trees in the repo: lyrics, translations, video versions. Translations are the deepest, it's : language/letter-group/artist/release/recording/; if we add a separate tree for translation maps, it'd have to be identical to that structure, plus that'd be additional work whenever one needs to add a map to go along with the translation... we could put that as part of LMML, but I'm not sure if the current format the mapping is in would collide with the metadata format or not... We could encode it in base64 or something like that, or at least escape newlines, but I feel like it would require more thorough planning/design and following testing. There could also be a sub-section after metadata to include the map, not sure how much better that would be, but something like this:

Translation
Translation
Translation

_______________
Metadata  value

_____________
  0 +1 2 3
   0,1  1 4 5

@defanor
Copy link

defanor commented Feb 6, 2022

I've replied by mail, but it still didn't appear here in about an hour after GitHub's mail server accepted it, so duplicating it from the web interface.

Edit: oh, I've replied to "From" address instead of "Reply-to", so the mail reply probably won't appear it all.


We've touched that subject before, in a couple of GitHub discussions for
this repository. Among other things, it was proposed to keep a single
deep hierarchy for artists-albums-songs, and only split it in the end:
would be as deep at some points, but we won't have to compare subtrees
(and it'd be harder to mess up). Though storing a translation and a
mapping in the same file makes sense too.

And I'm still not sure whether it'd be fitting to aim only one
translation per song.

but I'm not sure if the current format the mapping is in would collide
with the metadata format or not

Maybe it's the time to write down the format specification.

I think base64 or newline escapes would also make it less readable as a
textual document, which IIRC was the purpose of using a custom format
instead of XML, reStructuredText, or other existing ones. Although RST
field lists look quite similar to LMML metadata; maybe it's worth
considering switching to an RST subset, but perhaps it'd be a larger
change than the currently discussed ones (and might be better to handle
separately, not to mix it all up).

Though as long as we don't mess it up too badly, it must be possible to
do whatever is easier now, and automatically convert into another option
later.

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