Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Add encoding comment only if files syntax is not ok #6

Closed
wants to merge 2 commits into from

3 participants

Eduard Litau Jana Dvořáková Manuel Ryan
Eduard Litau

As discussed in this Stack Overflow article it is not best practice to put the encoding comment in each file, but only where it is needed. So here I skip adding of the comment if the files syntax is correct.

Jana Dvořáková

May be fine for english (and other ascii only languages), but not when you use non-ascii in comments for example... You would run into issues all the time using this approach. You would have to re-run magic_encoding all the time. I don't find this to be good solution.

Eduard Litau

I tend to use only english when programming, in code and comments (although my spoken language is not ascii compatible :smile:) - so for me it is kind of code smell if every file in a project has encoding comments. Furthermore it is possible to put non-ascii characters in comments, even without the encoding comment.

@jana4u: Which issue do you see coming using this approach?

Jana Dvořáková

It is not just comments where you can use non-ascii chars...

I don't think the behavior you suggest should be the default. It may suit someone who has 1 or 2 files in whole project that require magic comment, but not for people who have most files containing non-ascii - there is high probability that files currently without magic comment will require it soon as well.

I run current script once and I am set - no matter what characters I add to files later.

Jana Dvořáková

Maybe you could use some commandline switch to change behavior or add another executable to /bin to force suggested behavior for the people who would prefer it over current one.

Eduard Litau

Yes, a command line switch would be sufficient. But it seems like @m-ryan does not recognize this pull request, so i'm pleased with my fork. Otherwise I can submit a patch for the switch...

Eduard Litau

Or do you have push rights, @jana4u ?

Manuel Ryan
Owner

Sorry for the wait... I'm not going to take this one in as I fail to see any practical improvement or bugfix with this pull request. In my opinion, setting the comment on top of every ruby files doesn't hurt.

Anyone who thinks otherwise should use elitau's fork :)

Manuel Ryan m-ryan closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 8 additions and 1 deletion.
  1. +1 −1  bin/magic_encoding
  2. +7 −0 lib/magic_encoding.rb
2  bin/magic_encoding 100644 → 100755
View
@@ -2,6 +2,6 @@
# A simple tool to prepend magic comments for encoding to multiple ".rb" files
-require 'magic_encoding'
+require File.join(File.dirname(__FILE__), *%w[.. lib magic_encoding])
AddMagicComment.process(ARGV)
7 lib/magic_encoding.rb
View
@@ -2,6 +2,7 @@
# A simple library to prepend magic comments for encoding to multiple ".rb" files
+require 'open3'
module AddMagicComment
# Options :
@@ -29,6 +30,7 @@ def self.process(options)
extensions.each do |ext, comment_style|
rbfiles = File.join(directory ,'**', '*.'+ext)
Dir.glob(rbfiles).each do |filename|
+ next if syntax_ok?(filename)
file = File.new(filename, "r+")
lines = file.readlines
@@ -54,6 +56,11 @@ def self.process(options)
puts "Magic comments set for #{count} source files"
end
+ def self.syntax_ok?(filename)
+ stdin, stdout, stderr = Open3.popen3("ruby -c #{filename}")
+ stdout.read.include?("Syntax OK")
+ end
+
end
class String
Something went wrong with that request. Please try again.