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

Annotating haml and slim file #3889

Merged
merged 4 commits into from Dec 7, 2011
Merged

Annotating haml and slim file #3889

merged 4 commits into from Dec 7, 2011

Conversation

kielkowiczk
Copy link
Contributor

Hi,

Not sure why there is no way to annotate haml and slim file?
These template engines getting more and more popular, so why not allow to annotate them (with TODO, etc)?
Here is my proposal (code and tests) allowing annotation of these files.

Thanks for Your time:)

@josevalim
Copy link
Contributor

Yes, that would be nice but we should instead provide a mechanism for extension of the code annotator instead of simply hardcoding the logic. :(

@kielkowiczk
Copy link
Contributor Author

There are not that many engines, and they don't change that often, and it's not like new engines are emerging every week. Making mechanism for extension would nice (and challenging) but sound like over shoot (taking under consideration point listed above).

@josevalim
Copy link
Contributor

This argument is moot because it is like saying Rails can hardcode all the code to test-unit and rspec, without providing an API, because that's basically what people use it anyway.

@kielkowiczk
Copy link
Contributor Author

I see Your point, and You are probably right. Agree Rails should go in the direction of becoming more transparent and less hardcoded.

These commit ought to reflect change in Rails ecosystem, not revolutionizing Rails annotating system, just keeping it up to date:)

@josevalim
Copy link
Contributor

Ok. I will merge it but basically because I liked the test coverage. ;) Could you please also rename the test file to railties/test/application/rake/notes_test.rb? Also, if you want to send another pull request extracting these https://github.com/kielkowicz/rails/blob/8f61df0bcd0e84f26c6c41b98eeb2f7bb8d5ca4b/railties/test/application/rake_test.rb#L111-177 to a new file called railties/test/application/rake/migration_test.rb you will gain extra <3 .

@kielkowiczk
Copy link
Contributor Author

Done:)

josevalim added a commit that referenced this pull request Dec 7, 2011
Annotating haml and slim file
@josevalim josevalim merged commit 6867212 into rails:master Dec 7, 2011
@kielkowiczk
Copy link
Contributor Author

@sobrinho, It's exactly what @josevalim (btw, thanks!) suggested, making some sort of mechanism for extensions, so it would be "easier" (in the meaning not hardcoded in Rails) for other template engines to implement their annotation style.
Something to think for next iteration (or over weekend) ;)

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

Successfully merging this pull request may close these issues.

None yet

3 participants