scrottie / autobox-Core

Release history of autobox-Core
http://search.cpan.org/dist/autobox-Core/
23 stars 11 forks source link

Docs about split() is slightly wrong #17

Closed jarich closed 8 years ago

jarich commented 12 years ago

The documentation says:

Anything which takes a regular expression, such as split and m, must take it in the form of a compiled regex (qr//). Any modifiers can be attached to the qr normally.

This isn't true. The following code works:

 "the cat in the hat sat back"->split(" ")->reverse->say;

I like this better than:

split( qr/ / )

no matter how much Perl::Critic disapproves of strings containing nothing but whitespace. But, regardless, the documentation is currently wrong. How do we want to resolve this?

scottwalters commented 12 years ago

"Anything which takes a regular expression, such as split and m, must take it in the form of a compiled regex, when given a regex"? There was an excellent rant on p5p a couple of years back about how counterproductive it is to bloat docs with pedantry. That said, you're more than welcome to correct it how you see fit. If I see something go in that I have a philosophical objection to, I'll raise the point after the fact.

schwern commented 12 years ago

@scrottie I'm not sure which part you're calling out as pedantry, but I will note that it is non-obvious to a lot of users that these methods must take qr/foo/ and not bare /foo/. A line clarifying this is worth it, reinforced by all the examples. Some people learn by example, some people learn by rules.

@jarich I would suggest the docs supply a general rule, note there are exceptions, and then note the exceptions in place. Something like...

Anything which takes a regular expression, such as m() and grep(), must take it in the form of a compiled regex (qr//) unless stated otherwise.  Regex modifiers can be attached to the `qr` normally.  For example...

The split exceptions go in the split docs, and so on.

scottwalters commented 12 years ago

@schwern, the docs do already explain (as @jarich quoted them as saying) that things require regexes to be passed using qr//. What they don't explain, and she pointed out, is that you can also pass strings in come cases (such as to split). This makes the docs incorrect through omission, but only incorrect through omission about a possible way to do things, not about a way that things won't work or in a way that can cause things to go wrong. Adding text about it being possible to pass spaces in some cases won't hurt, but in general, I think it would be counter-productive to list everything that can be done and is supported (by virtual of the thin wrapper not breaking existing features of other things). That would repeat too much documentation and make the documentation ultimately less serviceable. I just wanted to go on record with that position on documentation, but I am still of my stated position on commits (go ahead).

scottwalters commented 12 years ago

I guess, if people want a rule of thumb from me, one rule of thumb might be this: document stuff that you care about, but don't try to be comprehensive for the sake of it. Re: "I would suggest the docs supply a general rule, note there are exceptions, and then note the exceptions in place.", agreed, generally, with that caveat. It could be noted that split() accepts a string as a pattern. Anyway, don't let me bog you down in additional detail and sap your energy. If anything really bugs me, I'll wait a year and then change it when no one is looking, in the unlikely case that I have somehow not forgotten about it ;)