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 n-level glossing and spacing parameters #1

Closed
wants to merge 0 commits into from

Conversation

betoma
Copy link
Contributor

@betoma betoma commented Apr 4, 2023

A handful of changes to allow for n-level style glossing (as in expex) and the ability to configure the spacing of certain elements in the gloss.

  • added nlevel parameter specifying whether the gloss is using n-level style or not. Defaults to false.
  • implemented line_styles parameter to allow specifying a list of styles for lines in the gloss without their own dedicated style parameter.
  • added header_text_style and translation_style parameters, corresponding to the existing source_text_style and morphemes_style parameters
  • made source_text and morphemes parameters optional, as they would not be present in an n-level style gloss
  • renamed additional_gloss_lines parameter to just gloss_lines, since for n-level glosses this is the only place gloss lines are provided
  • renamed gloss_lines function to build_gloss to avoid conflicts with the above parameter
  • removed requirement for all lines of a gloss to be equal in length. Glosses with uneven lengths will simply be blank on the shorter lines for the remainder of longer lines.
  • changed gloss function to use a ltr stack to display the gloss rather than using padding, allowing for the number of numbered glosses to appear on the same line as the first line of the gloss
  • added breakable parameter to indicate whether the gloss should be allowed to break across pages. Defaults to false.
  • added gloss_padding parameter to specify how far indented the gloss itself should be
  • added left_padding parameter to specify how far indented the number should be in numbered glosses. Does nothing for unnumbered glosses. Should be less than gloss_padding since this space is included in the space specified by gloss_padding.
  • added post_header_space and pre_translation_space parameters to specify how much space should appear between these elements and the gloss itself
  • renamed spacing_between_items to interword_spacing to more clearly distinguish its function compared to all the other spacing functions
  • added nogloss as a convenient alias for a space that can be used to make clear that a given item was intentionally given no corresponding text in a given gloss line

#let numbered_gloss = gloss.with(numbering: true)
#let numbered_gloss = gloss.with(numbering:true)

#let nogloss = " "
Copy link
Owner

Choose a reason for hiding this comment

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

Is this definition doing anything?

Copy link
Owner

Choose a reason for hiding this comment

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

Oh I see your explanation. It'd be good to use this in an example, to justify why it is there.

@neunenak
Copy link
Owner

neunenak commented May 7, 2023

Thanks a lot for adding explanatory text throughout the example document, that's a really helpful change.

morphemes: ([#smallcaps[conn]=him], [that.#dat.#sg], [time.#dat.#sg], [eat.they.shall]),
translation: "They shall celebrate him on that date",
)
```
Copy link
Owner

Choose a reason for hiding this comment

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

This code block needs to be updated with the actual custom values for spacing that you used.

@neunenak
Copy link
Owner

neunenak commented May 7, 2023

How hard would it be to make the n-level gloss behavior a separate named function from gloss itself? I don't like having to rename the additional_gloss_lines parameter to gloss_lines for the benefit of that functionality, or in general having effectively two ways to call the same function that each require omitting arguments applicable to the other?

style(styles => {
block(breakable: breakable)[
#stack(
dir:ltr,
Copy link
Owner

Choose a reason for hiding this comment

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

Mostly a note, this library needs to be able to support RTL text correctly to be useful to linguists working with Hebrew/Arabic/Farsi/etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not super familiar with how glosses for RTL languages are formatted (the only such glosses I've personally encountered used Latin transliterations) but I can look into detecting if the surrounding text is RTL and editing things based on that. I think even as-is it should still work fine with RTL text inside the gloss (though requiring the actual lists of words for each line to list the words in LTR order) but I haven't tested that.

Copy link
Owner

Choose a reason for hiding this comment

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

I think it's fine to not try to solve RTL text in this patch, it's just something I want to keep in mind.

@betoma
Copy link
Contributor Author

betoma commented May 7, 2023

How hard would it be to make the n-level gloss behavior a separate named function from gloss itself? I don't like having to rename the additional_gloss_lines parameter to gloss_lines for the benefit of that functionality, or in general having effectively two ways to call the same function that each require omitting arguments applicable to the other?

This is an interesting idea -- I principally implemented it as an optional argument because that's how it worked in expex, but while it would definitely take some work to separate out the nlevel functionality into a new function I think it would definitely work. Might even make the code more readable on the whole and allow for changes to be made to each function separately if something's easier for one type than another. I'd be up to separate it out into a separate function for sure if you think it's better.

@neunenak
Copy link
Owner

neunenak commented May 7, 2023

How hard would it be to make the n-level gloss behavior a separate named function from gloss itself? I don't like having to rename the additional_gloss_lines parameter to gloss_lines for the benefit of that functionality, or in general having effectively two ways to call the same function that each require omitting arguments applicable to the other?

This is an interesting idea -- I principally implemented it as an optional argument because that's how it worked in expex, but while it would definitely take some work to separate out the nlevel functionality into a new function I think it would definitely work. Might even make the code more readable on the whole and allow for changes to be made to each function separately if something's easier for one type than another. I'd be up to separate it out into a separate function for sure if you think it's better.

Yeah, I think it's better to optimize for readability/maintainability here, so go ahead and separate it out into a separate function.

neunenak added a commit that referenced this pull request Jul 4, 2023
@neunenak
Copy link
Owner

neunenak commented Jul 4, 2023

@betoma in the interest of fixing the most pressing bug (the numbered gloss number appearing on the wrong line), and being able to submit this as a typst package, I've manually copied some of the code and documentation improvements in this PR to the master branch. I will try to move over the rest of the improvements + implement n-level glossing as a separate function in the next several days.

@betoma
Copy link
Contributor Author

betoma commented Sep 18, 2023

@neunenak Thanks so much for doing that! I haven't had time to fix up this code to separate out functions and such so I appreciate you grabbing the stuff that's most useful.

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.

None yet

2 participants