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

Only call 'noindent' if language is English in book commands #1991

Closed
wants to merge 1 commit into from

Conversation

jodros
Copy link
Contributor

@jodros jodros commented Feb 6, 2024

Since it's a English feature it shouldn't be called if document.language isn't English as well...

@jodros jodros requested a review from alerque as a code owner February 6, 2024 03:28
@jodros jodros changed the title Only call 'noindent' if language is English Only call 'noindent' if language is English in book commands Feb 6, 2024
@jodros jodros marked this pull request as draft February 6, 2024 03:34
@jodros jodros marked this pull request as ready for review February 6, 2024 03:49
Copy link
Member

@alerque alerque left a comment

Choose a reason for hiding this comment

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

No, sorry that source comment is misleading. This is not an "English" feature, it is quite a common default for many languages that use Latin based orthographies. For example Turkish ;-)

The more relevant point is that there are a handful of languages that specifically eschew this: French being a notable one. I think the correct approach here will be to make this behavior have a per-language default. That isn't easy to do right no because of the chaotic overlap in scopes for the language modules, but I'm actually refactoring that right now and am hopeful we'll have a better place to "hang this hat" soon. I'll leave this PR around as a reminder of the issue and we'll update it to to the right thing when there is a sensible way to do so.

@alerque alerque marked this pull request as draft February 6, 2024 10:04
@Omikhleia
Copy link
Member

Omikhleia commented Feb 6, 2024

The more relevant point is that there are a handful of languages that specifically eschew this:

And I might be mistaken, but usage depends on conventions that are not strictly language-specific (i.e. there could be a language-specific default, but it would likely need to be something the user can configure)
(As an aside note, in a styling paradigm such as your @jodros, or mine, this could be left to a styling decision)

@jodros
Copy link
Contributor Author

jodros commented Feb 6, 2024

I'm actually refactoring that right now and am hopeful we'll have a better place to "hang this hat" soon. I'll leave this PR around as a reminder of the issue and we'll update it to to the right thing when there is a sensible way to do so.

Right, things aren't as simple as they look sometimes...

(As an aside note, in a styling paradigm such as your @jodros, or mine, this could be left to a styling decision)

yeah, I thought about that, and I really think that we should discuss further the implementation of a basic styling paradigm in SILE, this is essencial for typesetting!

@Omikhleia
Copy link
Member

An interesting discussion on the topic: https://tex.stackexchange.com/a/39232

(The whole discussion mentions typographers such as Bringhurst, Tschichold; and the French Imprimerie Nationale -- all sorts of lectures I personally like)

There could be a language-specific default, thus...
--> Linking to #2017 as we might need some other language-specific customization there too.

@Omikhleia Omikhleia added enhancement Software improvement or feature request question Ask for advice or investigate solutions labels May 7, 2024
@Omikhleia
Copy link
Member

This old PR now has conflicts, and last discussions pointed it would not occur as-is.
I am closing it, but please feel free to re-open a dedicated issue and/or PR if a different approach should be discussed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Software improvement or feature request question Ask for advice or investigate solutions
Projects
Development

Successfully merging this pull request may close these issues.

3 participants