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

wants to merge 2 commits into

3 participants


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.


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.


I tend to use only english when programming, in code and comments (although my spoken language is not ascii compatible 😄) - 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?


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.


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.


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...


Or do you have push rights, @jana4u ?


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 :)

@mryan43 mryan43 closed this May 7, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment