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 latexpdf support for Greek as main language (via xelatex use) #6711

Merged
merged 3 commits into from
Oct 8, 2019

Conversation

jfbu
Copy link
Contributor

@jfbu jfbu commented Sep 30, 2019

Closes: #6710

Feature or Bugfix

  • Bugfix

@jfbu jfbu added this to the 2.2.1 milestone Sep 30, 2019
@codecov
Copy link

codecov bot commented Sep 30, 2019

Codecov Report

Merging #6711 into 2.2.1 will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##            2.2.1    #6711      +/-   ##
==========================================
+ Coverage   83.78%   83.78%   +<.01%     
==========================================
  Files         269      269              
  Lines       40970    40981      +11     
  Branches     5998     5999       +1     
==========================================
+ Hits        34325    34336      +11     
  Misses       5321     5321              
  Partials     1324     1324
Impacted Files Coverage Δ
tests/test_build_latex.py 95.38% <100%> (+0.04%) ⬆️
sphinx/builders/latex/__init__.py 81.36% <100%> (+0.14%) ⬆️
sphinx/writers/latex.py 81.93% <100%> (+0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 94a0979...6a1ea84. Read the comment docs.

@jfbu
Copy link
Contributor Author

jfbu commented Oct 1, 2019

I have squashed the commits. Python 3.6 mypy testing fails for reasons unrelated to this PR.

Copy link
Member

@tk0miya tk0miya left a comment

Choose a reason for hiding this comment

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

LGTM with nits.

Use ``xelatex`` (and LaTeX package ``xeCJK``) by default for Chinese
documents.

.. versionchanged:: 2.2.1
Copy link
Member

Choose a reason for hiding this comment

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

Now, we are using x.y.Z branch for urgent bugfix release. I think 2.0 branch is appropriate for this fix.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, sorry. I just read #6710 and understand this is a bug. Okay, go with 2.2.1.

@@ -418,6 +418,8 @@ def default_latex_engine(config: Config) -> str:
return 'platex'
elif (config.language or '').startswith('zh'):
return 'xelatex'
elif (config.language or '') == 'el':
Copy link
Member

Choose a reason for hiding this comment

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

or '' is not needed for this case. It is useful to call methods of string object even if config.language is None. But this line simply compares with string.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

blindly copying pasting is fine, but thinking is better, thanks for the explanation :) will fix

@tk0miya
Copy link
Member

tk0miya commented Oct 6, 2019

I have squashed the commits. Python 3.6 mypy testing fails for reasons unrelated to this PR.

I fixed the error in latest 2.2.1 branch today. Please merge it into this. Then error will go away.

@jfbu
Copy link
Contributor Author

jfbu commented Oct 8, 2019

Thanks for reviewing @tk0miya now merging into 2.2.1

@jfbu jfbu merged commit d000de6 into sphinx-doc:2.2.1 Oct 8, 2019
@jfbu jfbu deleted the latex_greekfix branch October 8, 2019 12:21
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants