search

mryan43 / magic_encoding

Easily manage magic comments for encoding for your ruby 1.9 applications
MIT License
243 stars 27 forks source link

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

Closed elitau closed 12 years ago

elitau commented 12 years ago

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.

jana4u commented 12 years ago

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.

elitau commented 12 years ago

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?

jana4u commented 12 years ago

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.

jana4u commented 12 years ago

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.

elitau commented 12 years ago

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

elitau commented 12 years ago

Or do you have push rights, @jana4u ?

mryan43 commented 12 years ago

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