Add "encoding" configuration. (3rd version)

[koron]

Encoding of input files should be determined by site settings (configuration).

Currently, it is determined by OS or environment settings implicitly.
This patch make it configurable.

(This request is revised version of #1365, #1366)

[kelvinst]

Hi @koron.
I have some suggestions that I will make in code comments. But in short, I think this is a G-R-E-A-T and simple idea (I'm even wary if this works for us, I mean, why did anyone have this idea before??). @parkr, what do you think about?

[koron]

@kelvinst Thank you for your review.

Use spaces around the = operator when assigning default values to method parameters:

I have fixed.

[shigeya]

+1 to include patch. I found encoding issue and this patch should solve my issue too. I planned to create a patch if I can't find any.

[crazymaster]

👍

[mattn]

+1

[parkr]

Before we can accept this patch, we need tests to make sure we don't break it in the future.

[parkr]

Why not set your system LANG? This is how one gets around this problem.

[koron]

Why not set your system LANG?

There are two reason.

1. Encoding should depend on contents, not system wide setting.
2. LANG wasn't working with Ruby 2.0.0 on Windows.

[shigeya]

I was trying to create test to pass, but unfortunately, in my environment with ruby 1.9.3, the test does not pass. I pulled upstream and ran test, still failing. And it's related to encoding. Anything you know?

  1) Failure: test: yaml front-matter should not parse if there is encoding error in file. (TestConvertible) [/Users/shigeya/git/jekyll/test/test_convertible.rb:44]:
<{}> expected but was
<{"test"=>"good"}>.

[koron]

@shigeya Please try a my experimental patch: https://github.com/koron/jekyll/commit/4435c565d4097ea2a5f0dad10df88ff1e6c244aa

[shigeya]

@koron I don't like to introduce default_external config even in a test. How about this?

shigeya@6658a2f

This commit introduce optional parameter to Convertible.read_yaml too

[shigeya]

I've sent a pull request on that fix.

[koron]

@shigeya 👍

But, It seems to conflict with changes of this pull request 😉

[shigeya]

You're right. I think you can make the default to Convertible.read_yaml read from the configured parameter, not {}.
And the test side of above patch will pass regardless of the option given in the test case currently failing.

[shigeya]

I have added two commits to show my intention, on top of @koron's branch: https://github.com/shigeya/jekyll/tree/config-encoding-and-yaml-opts

As @parkr suggested, we need to have tests for the configuration part.

[mattr-]

The Encoding class is not available in 1.8.x. Jekyll will be maintaining 1.8.x compatibility until version 2.0.

[shigeya]

Yes. And the changes on this branch take care of that. Thus, 1) the encoding related test won't run on 1.8.7, and 2) unless user specifically configure to pass encoding parameter(s), it will not see any encoding related requests. In my environment, all tests pass without any issue on 1.8.7-p371, 1.9.3-p448 and 2.0.0-p247.

Part of the problem of the encoding is, UTF-8 file is not readable by default encoding (ASCII-8BIT) on 1.9.x and beyond. The solution like this patch, or, global configuration of external encoding config (wrapped with >=1.9.3) conditional) is the only solution.

[mattn]

We must know most of users who uses multi-byte with ruby2.0 on windows are lost to get the way to run jekyll. :-(

[mattr-]

So I ran this pull request on Jekyll's site with ruby 1.8.7 and added the following to _config.yml

encoding: utf-8

and this is what I got when I tried to generate the site.

$ jekyll build
Configuration file: /Users/matt/Code/jekyll/site/_config.yml
error: uninitialized constant Jekyll::Site::Encoding. Use --trace to view backtrace

this makes Jekyll incompatible with ruby 1.8.7 which is not acceptable at this point in time as we've committed to maintaining backward compatibility with that version of Ruby until Jekyll 2.0. This incompatibility will need to be fixed before this pull request can be merged.

[shigeya]

Oh, I overlooked the issue. I (or @koron ?) will provide fix to that. (+ tests on that too)

[shigeya]

I have added a commit shigeya@491fc04 to the branch. Just pass encoding parameter as a string. We can wrap it with RUBY_VERSION variable if we prefer (may be little performance improvement to fetch the object itself.)

@mattr, please confirm you can build without problem. (I also curious why Encoding.find gets called. Did you configure encoding variable?)

We need test. @koron, Do you wiling to write tests? if not, I can write some.

[koron]

We need test. @koron, Do you wiling to write tests? if not, I can write some.

No, I cann't now or soon. Please write it.

[shigeya]

I have created another pull request #1449 for this, to avoid confusion. We need tests, but need some input to how to implement.