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

Please don't create ending whitespace #13

Closed
david-a-wheeler opened this issue May 9, 2017 · 6 comments
Closed

Please don't create ending whitespace #13

david-a-wheeler opened this issue May 9, 2017 · 6 comments

Comments

@david-a-wheeler
Copy link

By policy our project doesn't allow files with whitespace errors, as determined by "git diff --check".

Unfortunately, "sync" creates .yml files with trailing whitespace. It's possible to fix it, but it's a pain, and I suspect many projects have this rule.

Could sync stop creating trailing whitespace?

Thanks so much! I only just started, but this does look like an interesting service.

FYI, here's the documentation from git diff --check:

       --check
           Warn if changes introduce whitespace errors. What are considered
           whitespace errors is controlled by core.whitespace configuration.
           By default, trailing whitespaces (including lines that solely
           consist of whitespaces) and a space character that is immediately
           followed by a tab character inside the initial indent of the line
           are considered whitespace errors. Exits with non-zero status if
           problems are found. Not compatible with --exit-code.
@MichaelHoste
Copy link
Member

Do you have an example of an .yml file with trailing whitespace?

If I'm right, trailing whitespaces are only added by the YAML library if the value of a key is empty.

Example with no empty value (and no trailing spaces):

irb(main):003:0> { :a => 'a', :b => 'b' }.to_yaml
=> "---\n:a: a\n:b: b\n"

Example with empty value (and trailing space):

irb(main):007:0> { :a => nil, :b => 'b' }.to_yaml
=> "---\n:a: \n:b: b\n"

As you can see, there is a trailing space after "a:"

I don't know why the YAML library do that, and, of course, that's something we could manage to remove within the translation gem.

But isn't it something that we should leave the YAML library deal with?

@david-a-wheeler
Copy link
Author

Yes, this only happens with empty keys.

Hmm.. sounds like I should report this to the YAML library instead.

@MichaelHoste
Copy link
Member

MichaelHoste commented May 10, 2017

sounds like I should report this to the YAML library instead.

Since you're not the first one to make that observation about the trailing whitespace, I already tried to do that but I don't know exactly where to report it.

Is it on:

or

After wandering around on the repositories, I found out that this issue is already reported on libYAML: yaml/libyaml#46

david-a-wheeler added a commit to coreinfrastructure/best-practices-badge that referenced this issue May 10, 2017
The "translation:sync" task syncs up the translations, but uses the usual
YAML writer, which writes out trailing whitespace.  It should not do that,
and the trailing whitespace causes later failures in testing.
Problem already reported:
- translation/rails#13
- yaml/libyaml#46

For now, we'll run this enhancement to solve the problem.

Signed-off-by: David A. Wheeler <dwheeler@dwheeler.com>
@david-a-wheeler
Copy link
Author

Thanks, great info.

Okay, I've solved it for us by adding this to our "lib/tasks/default.rake" - you might find it useful to document this.

# Remove trailing whitespace after running "translation:sync".
# The "translation:sync" task syncs up the translations, but uses the usual
# YAML writer, which writes out trailing whitespace.  It should not do that,
# and the trailing whitespace causes later failures in testing.
# Problem already reported:
# - https://github.com/aurels/translation-gem/issues/13
# - https://github.com/yaml/libyaml/issues/46
# We'll run this enhancement to solve the problem.
Rake::Task['translation:sync'].enhance do
  puts 'Removing bogus trailing whitespace (bug workaround).'
  sh "cd config/locales/ && sed -i -e 's/ $//' *.yml && cd ../.."
end

@dankohn dankohn mentioned this issue Jun 29, 2017
MichaelHoste added a commit that referenced this issue Sep 10, 2019
 * Option 'yaml_line_width' to wrap YAML files + tests: #19
 * Remove extra-whitespace in YAML files (issue with libYAML) + tests: #13
 * Replace <<EOS by <<-EOS for better highlight.
@MichaelHoste
Copy link
Member

It's finally fixed here: 46ec27b

And part of 1.19: https://github.com/translation/rails/releases/tag/v1.19

@david-a-wheeler
Copy link
Author

Yay! Thanks!

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

No branches or pull requests

2 participants