Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Fixed mailto for SafeBuffer#gsub already in 3-0-stable #1543

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants
Member

arunagw commented Jun 8, 2011

This makes test/template/url_helper_test.rb happy.

Already in 3-0-stable.

Member

sikachu commented Jun 8, 2011

Why does this need to fix the test too?

Contributor

cmeiklejohn commented Jun 8, 2011

This is correct, test prior to SafeBuffer commit was encoding escaped string:

<a href=\"mailto:me@domain.com\">My email<\/a>

Now it's encoding:

<a href="mailto:me@domain.com">My email</a>
Member

NZKoz commented Jun 8, 2011

I'm sorry if I'm being a little dense but I don't actually see the problem this is intent on fixing?

Contributor

cmeiklejohn commented Jun 8, 2011

This fixes the 3 failing tests in test/template/url_helper_test.rb

Member

arunagw commented Jun 8, 2011

  3) Error:
test_mail_to_with_replace_options(UrlHelperTest):
TypeError: Cannot modify SafeBuffer in place
    /Users/arun/checkouts/rails/activesupport/lib/active_support/core_ext/string/output_safety.rb:121:in `gsub!'
    /Users/arun/checkouts/rails/actionpack/lib/action_view/helpers/url_helper.rb:501:in `mail_to'
    test/template/url_helper_test.rb:401:in `test_mail_to_with_replace_options'
    /Users/arun/checkouts/rails/activesupport/lib/active_support/testing/setup_and_teardown.rb:67:in `__send__'
    /Users/arun/checkouts/rails/activesupport/lib/active_support/testing/setup_and_teardown.rb:67:in `run'
    /Users/arun/checkouts/rails/activesupport/lib/active_support/callbacks.rb:408:in `_run_setup_callbacks'
    /Users/arun/checkouts/rails/activesupport/lib/active_support/callbacks.rb:81:in `send'
    /Users/arun/checkouts/rails/activesupport/lib/active_support/callbacks.rb:81:in `run_callbacks'
    /Users/arun/checkouts/rails/activesupport/lib/active_support/testing/setup_and_teardown.rb:65:in `run'

Member

NZKoz commented Jun 8, 2011

Why do the assertions change though? The output shouldn't differ just to support this change? it should be identical right? The " is actually important.

Member

sikachu commented Jun 8, 2011

I don't think that's a fix if you have to change the test.

Member

arunagw commented Jun 8, 2011

@NZKoz I think then we need to look for code again. Trying to fix it again.

Member

sikachu commented Jun 8, 2011

I've already fixed it in #1546.

Member

arunagw commented Jun 8, 2011

@sikachu Works well for me. Can you please take care for 3-0-stable branch. These changes required there also.

Closing this.!

Cheers,
Arun

@arunagw arunagw closed this Jun 8, 2011

jake3030 pushed a commit to jake3030/rails that referenced this pull request Jun 28, 2011

Use table prefix and suffix for schema_migrations index.
[#1543 state:committed]

Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>

jake3030 pushed a commit to jake3030/rails that referenced this pull request Jun 28, 2011

Add tests for scoping schema_migrations index by global table prefix …
…and suffix

[#1543 state:committed]

Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment