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

Change language priority for environment variables #123

Merged
merged 8 commits into from
Jul 23, 2020

Conversation

columbarius
Copy link
Contributor

Regarding the language priority discussed in #122 and tldr-pages/tldr#4101 I have implemented a solution. My suggestion would be a priority like that

TLDR_LANGUAGE LANG LANGUAGES Result
fr de it,fr,de fr
! de it,fr,de de,it,fr,en
! ! it,fr,de it,fr,de,en
! de ! de,en

, while ! is unset.

I'm also interested, where LANGUAGES originates from. While I couldn't find a notion of it in the archwiki, or at gnu.org, I found the LANGUAGE environment variable instead.

Since this topic is still in discussion I'm submitting this as a draft in case it could be useful.

@zlatanvasovic
Copy link
Contributor

Does it agree with the specification, though? I think it should be fr, en in the first row. Either way, I think it'd be useful if you posted your ideas in tldr-pages/tldr#4101.

@MasterOdin
Copy link
Collaborator

I'm also interested, where LANGUAGES originates from. While I couldn't find a notion of it in the archwiki, or at gnu.org, I found the LANGUAGE environment variable instead.

Because I'm bad at typing and it should be LANGUAGE. Opening a PR now for that.

@columbarius
Copy link
Contributor Author

columbarius commented Jun 20, 2020

Does it agree with the specification, though? I think it should be fr, en in the first row.

I read TLDR_LANGUAGE in the same role of the --language flag to replace all other languages.

Either way, I think it'd be useful if you posted your ideas in tldr-pages/tldr#4101.

All right, then I will do that.

Because I'm bad at typing and it should be LANGUAGE. Opening a PR now for that.

Ok, thanks.

@columbarius
Copy link
Contributor Author

I've updated the language handling to fit my proposal in tldr-pages/tldr#4092 (comment) to test it for convenience.

@columbarius columbarius marked this pull request as ready for review July 22, 2020 12:15
@columbarius
Copy link
Contributor Author

Since tldr-pages/tldr#4101 was merged I changed this from Draft to PR.

@columbarius
Copy link
Contributor Author

Should I bump client_specification to 1.3 in this PR, or would this be done in an extra commit?

@zlatanvasovic
Copy link
Contributor

@columbarius go ahead, nice catch!

@columbarius
Copy link
Contributor Author

client_specification bumped. I guess, I should have written this earlier to trigger a notification?

@zlatanvasovic
Copy link
Contributor

No need, I am just leaving the final merge decision to @MasterOdin.

@MasterOdin MasterOdin merged commit baf2a7d into tldr-pages:master Jul 23, 2020
@MasterOdin
Copy link
Collaborator

Thanks for this. Still some refactoring I'd like to do in the next week or so before cutting a new release.

@columbarius
Copy link
Contributor Author

Ok, thanks for merging.

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.

3 participants