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

Source file encoding specifier support for template #2233

Merged
merged 3 commits into from
Feb 18, 2020

Conversation

AoiMoe
Copy link
Contributor

@AoiMoe AoiMoe commented Feb 17, 2020

Here is a small patch to add encoding handling support for template.

How to test

  1. save as ~/.config/rebar3/templates/00latin1.template in Latin-1:
%% coding:latin-1
{description, "±¼"}.
  1. save as ~/.config/rebar3/templates/00utf8.template in UTF-8:
%% coding:utf-8
{description, "こんにちは"}.
  1. run rebar3 new
$ ./rebar3 new
00latin1 (custom): ±¼
00utf8 (custom): こんにちは
...

@ferd
Copy link
Collaborator

ferd commented Feb 17, 2020

I get why that would be desirable. However I think for this to be acceptable we need:

  1. Tests (see https://github.com/erlang/rebar3/blob/master/CONTRIBUTING.md)
  2. To follow the Erlang character set encoding rules defined at https://erlang.org/doc/reference_manual/character_set.html#source-file-encoding

Part 1 is simply not done; Part 2 appears to be done for templates, though I'm not sure why the shell module has been modified there. I'd revert that one since the defaults in the string utils module should ensure it keeps working.

Let me know if you'd need help writing the tests and figuring that one out.

@ferd ferd added the awaiting update requiring action from submitter label Feb 17, 2020
@AoiMoe AoiMoe force-pushed the AoiMoe/encoding_support_for_template branch 2 times, most recently from 8c8819b to cfbf9f4 Compare February 18, 2020 05:41
@AoiMoe AoiMoe force-pushed the AoiMoe/encoding_support_for_template branch from cfbf9f4 to 5a377a7 Compare February 18, 2020 06:07
@AoiMoe
Copy link
Contributor Author

AoiMoe commented Feb 18, 2020

Sorry, I committed changes with --amend and pushed with -f by mistake.
I restored the last reviewed commit from reflog and pushed it forcibly again.

@AoiMoe
Copy link
Contributor Author

AoiMoe commented Feb 18, 2020

Part 1 is simply not done

I added simple tests for rebar_templater:consult_template/3.

Part 2 appears to be done for templates, though I'm not sure why the shell module has been modified there. I'd revert that one since the defaults in the string utils module should ensure it keeps working.

Okay, I just reverted it and applied simplified changes.

Thanks for your comments.

@ferd ferd merged commit a1ae6b9 into erlang:master Feb 18, 2020
@AoiMoe AoiMoe deleted the AoiMoe/encoding_support_for_template branch February 19, 2020 02:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting update requiring action from submitter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants