Rails core - Jul 2007 - Could a core member take a moment to commit these patches?

I''ve got a couple small patches that are quite helpful and minimally
invasive that I''d love to get out of my vendor/rails so I can start
work on new patches with a clean edge rails (without checking out
another copy).  If any core member has time to take a look at these
and either commit these or give me some feedback I''d really appreciate
it:

http://dev.rubyonrails.org/ticket/8683

Koz looked at this one and asked for a test 3 weeks ago, which I
provided, but I haven''t heard back.  This one is particularly useful
for development because without it, Template Missing errors don''t
actually say what template was requested (broken since the addition of
multiple controller view paths).

http://dev.rubyonrails.org/ticket/8173

This one''s been out there for several months with nary a comment
(other than "not so tiny", which I dispute).  It makes XML testing
with assert_select work.  The only condition under which it would
break someone''s existing tests would be if they were sending content
type application/xhtml+xml (which no one does because IE doesn''t
understand it) AND they were producing invalid XHTML (an unlikely
combination to say the least).  So it seems like a no-brainer to me.
Jamis even blogged about a workaround
(http://weblog.jamisbuck.org/2007/1/4/assert_xml_select), so it''s
surprising there''s been no comment.

Thanks in advance,
Gabe da Silveira

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

On 7/12/07, Gabe da Silveira <gabriel.d@gmail.com>
wrote:
>
> I''ve got a couple small patches that are quite helpful and
minimally
> invasive that I''d love to get out of my vendor/rails so I can
start
> work on new patches with a clean edge rails (without checking out
> another copy).  If any core member has time to take a look at these
> and either commit these or give me some feedback I''d really
appreciate
> it:
>
> http://dev.rubyonrails.org/ticket/8683
>
> Koz looked at this one and asked for a test 3 weeks ago, which I
> provided, but I haven''t heard back.  This one is particularly
useful
> for development because without it, Template Missing errors don''t
> actually say what template was requested (broken since the addition of
> multiple controller view paths).
Applied. Thanks!

> http://dev.rubyonrails.org/ticket/8173
>
> This one''s been out there for several months with nary a comment
> (other than "not so tiny", which I dispute).  It makes XML
testing
> with assert_select work.  The only condition under which it would
> break someone''s existing tests would be if they were sending
content
> type application/xhtml+xml (which no one does because IE doesn''t
> understand it) AND they were producing invalid XHTML (an unlikely
> combination to say the least).  So it seems like a no-brainer to me.
> Jamis even blogged about a workaround
> (http://weblog.jamisbuck.org/2007/1/4/assert_xml_select), so it''s
> surprising there''s been no comment.
assert_select and xml is way outside my comfort zone so I''ll pass on
this one.   Anyone else have any comments on it?

If you can get three regular contributors to report on the utility of
the patch, and the implementation, I''d feel more comfortable applying
it.

-- 
Cheers

Koz

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

On 7/11/07, Michael Koziarski <michael@koziarski.com>
wrote:
>
> Applied. Thanks!
Thank you!
> > http://dev.rubyonrails.org/ticket/8173
> assert_select and xml is way outside my comfort zone so I''ll pass
on
> this one.   Anyone else have any comments on it?
>
> If you can get three regular contributors to report on the utility of
> the patch, and the implementation, I''d feel more comfortable
applying
> it.
I know there''s gotta be people out there bitten by this one.  If your
XML schema contains auto-closing HTML tags (such as <area>) then
testing with assert_select becomes unusable.  The patch is so simple,
if you''re doing RESTful APIs and like the power of assert_select,
please take 5 minutes to check it out.  It''ll save you a lot of ugly
hacks.

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

On 7/12/07, Gabe da Silveira <gabriel.d@gmail.com>
wrote:
>
> On 7/11/07, Michael Koziarski <michael@koziarski.com> wrote:
> >
> > Applied. Thanks!
>
> Thank you!
>
> > > http://dev.rubyonrails.org/ticket/8173
>
> > assert_select and xml is way outside my comfort zone so I''ll
pass on
> > this one.   Anyone else have any comments on it?
> >
> > If you can get three regular contributors to report on the utility of
> > the patch, and the implementation, I''d feel more comfortable
applying
> > it.
>
> I know there''s gotta be people out there bitten by this one.  If
your
> XML schema contains auto-closing HTML tags (such as <area>) then
> testing with assert_select becomes unusable.  The patch is so simple,
> if you''re doing RESTful APIs and like the power of assert_select,
> please take 5 minutes to check it out.  It''ll save you a lot of
ugly
> hacks.
I''ve been bitten by this before, and would love to see assert_select
made more friendly for xml.

- Rob
-- 
http://robsanheim.com

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

How are css selectors useful when testing xml? Do you just use the
element names?

As for the patch, you should probably use the lookup function in
mime.rb instead of regexp matching. That''ll remove the false positives

On 7/13/07, Rob Sanheim <rsanheim@gmail.com>
wrote:
>
> On 7/12/07, Gabe da Silveira <gabriel.d@gmail.com> wrote:
> >
> > On 7/11/07, Michael Koziarski <michael@koziarski.com> wrote:
> > >
> > > Applied. Thanks!
> >
> > Thank you!
> >
> > > > http://dev.rubyonrails.org/ticket/8173
> >
> > > assert_select and xml is way outside my comfort zone so
I''ll pass on
> > > this one.   Anyone else have any comments on it?
> > >
> > > If you can get three regular contributors to report on the
utility of
> > > the patch, and the implementation, I''d feel more
comfortable applying
> > > it.
> >
> > I know there''s gotta be people out there bitten by this one. 
If your
> > XML schema contains auto-closing HTML tags (such as <area>) then
> > testing with assert_select becomes unusable.  The patch is so simple,
> > if you''re doing RESTful APIs and like the power of
assert_select,
> > please take 5 minutes to check it out.  It''ll save you a lot
of ugly
> > hacks.
>
> I''ve been bitten by this before, and would love to see
assert_select
> made more friendly for xml.
>
> - Rob
> --
> http://robsanheim.com
>
> >
>

-- 
Cheers

Koz

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

Gabe da Silveira wrote:
> I know there''s gotta be people out there bitten by this one.
Yes. The case I got bitten by is the <link> element: in HTML it''s
self-closing, in feed XML it is not.

I currently use the method from 
http://weblog.jamisbuck.org/2007/1/4/assert_xml_select, which defines a 
separate assert method for the XML case.

It looks like the original assert_select plug-in may have had something 
similar in the form of assert_select_feed (it still features in Edge 
comments, but the code doesn''t seem to have survived.)

I don''t know of any reason not to fix up the underlying response as in 
your patch, though. Anyone? I do think the method name seems wrong now - 
it''s not really an html_document you''re returning. Should the
fix really
be applied in response_from_page_or_rjs?

I note that the patch fixes css_select too, as well as assert_select - 
it currently suffers from the same set of XML problems.

  --Anthony.


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

On 7/12/07, Michael Koziarski <michael@koziarski.com>
wrote:
>
> How are css selectors useful when testing xml? Do you just use the
> element names?
element names, attributes, child, ancestor, sibling, pseudo-classes
like :empty, etc.  Pretty much anything but .class and #id selectors
are widely useful.  Think about testing your XML with regexps (yech!)
or installing a parser.  assert_select provides an amazingly powerful
syntax for a non-trivial problem domain.  If only web browsers
supported all those selectors!
> As for the patch, you should probably use the lookup function in
> mime.rb instead of regexp matching. That''ll remove the false
positives
Agreed, this will be an improvement.  I''m checking out mime.rb now to
figure out how to do this.

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

On 7/14/07, Gabe da Silveira <gabriel.d@gmail.com>
wrote:
> On 7/12/07, Michael Koziarski <michael@koziarski.com> wrote:
> > As for the patch, you should probably use the lookup function in
> > mime.rb instead of regexp matching. That''ll remove the false
positives
>
> Agreed, this will be an improvement.  I''m checking out mime.rb now
to
> figure out how to do this.
Okay, I''ve gone in and made the patch using the lookup function.
However, after looking closer at the situation, I believe the original
patch is better for a number of reasons which I''ve detailed in the
patch comments.

I don''t believe there is any possibilty of a false positive, because
the ONLY type that should be parsed as HTML is text/html.  Everything
else (including XHTML) should be parsed as XML.  There are numerous
types of XML, including user-defined XML formats, all of which use a
standard mime_type convention of application/type+xml, so there is no
way to get around the use of a regexp.  Meanwhile, the lookup function
requires Rails-known mime types, even though the developer can specify
any content-type he wants.

So far it seems a few people are interested in the patch.  I haven''t
marked it verified, is the rule for that you have to get the explicit
three +1s?  Should I go on rails-contrib?  I would think the mailing
list gets more eyes, but I''m ready to do whatever it takes to get this
puppy committed.

-- 
Gabe da Silveira
http://darwinweb.net

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---