Extension docs: Adjusting `.../samples.html` to have more readable diffs.

Extension docs: Adjusting `.../samples.html` to have more readable diffs.

BUG=92148
TEST=Read the docs. Do they make sense?

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=96361

[Use mkwst_at_chromium.org plz., 2011-08-09 11:31:22]

Hey Pam, Arne, can you take a look at this CL?  I'm adjusting `samples.html` to
autogenerate code that diffs a bit better (linebreaks), just so it's clear
what's being added.

It's not entirely pretty; alternative implementation suggestions welcome.

Thanks!

-Mike

[Pam (message me for reviews), 2011-08-09 11:52:06]

Another option: use JSON.stringify, then split the text result on
quote-comma-quote... as long as it's not inside a string literal. You could test
for " in the split piece to be sure. Maybe more trouble than it's worth.

http://codereview.chromium.org/7601024/diff/1/chrome/common/extensions/docs/s...
File chrome/common/extensions/docs/static/samples.html (right):

http://codereview.chromium.org/7601024/diff/1/chrome/common/extensions/docs/s...
chrome/common/extensions/docs/static/samples.html:67: } else {
I'm not a JS expert, but should this be out one level? That is, fall back to
JSON.stringify() if we find any key in obj that doesn't match hasOwnProperty(),
not just if it's not a string.

[Use mkwst_at_chromium.org plz., 2011-08-09 11:57:23]

On 2011/08/09 11:52:06, Pam wrote:
> Another option: use JSON.stringify, then split the text result on
> quote-comma-quote... as long as it's not inside a string literal. You could
test
> for " in the split piece to be sure. Maybe more trouble than it's worth.

Yeah. I didn't do that just because I didn't want to think about all the ways
that quotes could be escaped. I think `","` is probably safe to split on, but
this was simpler for my tiny brain to plod through.

I'm hoping Arne has a better idea. :)

[Use mkwst_at_chromium.org plz., 2011-08-09 11:57:32]

http://codereview.chromium.org/7601024/diff/1/chrome/common/extensions/docs/s...
File chrome/common/extensions/docs/static/samples.html (right):

http://codereview.chromium.org/7601024/diff/1/chrome/common/extensions/docs/s...
chrome/common/extensions/docs/static/samples.html:67: } else {
On 2011/08/09 11:52:06, Pam wrote:
> I'm not a JS expert, but should this be out one level? That is, fall back to
> JSON.stringify() if we find any key in obj that doesn't match
hasOwnProperty(),
> not just if it's not a string.

If it's a property that the object doesn't itself have, then it's a property of
it's prototype (methods like `toString` or the like). It's perfectly reasonable
for an object to have inherited properties that we wouldn't want to print (I
guess...).

In this particular scenario, it makes no difference at all, as we create an
object literal that has no properties to speak of. But it's best practice, so...

[Pam (message me for reviews), 2011-08-09 12:50:56]

> In this particular scenario, it makes no difference at all, as we create an
> object literal that has no properties to speak of. But it's best practice,
so...

What about defining a naive toJSON method on the search_data object and using
plain JSON.stringify?

Or, I see that "The optional space parameter [to stringify] produces a
stringification of the value that is filled with line breaks and indentation to
make it easier to read." The example given at
https://github.com/douglascrockford/JSON-js/blob/master/json.js is

text = JSON.stringify(['e', {pluribus: 'unum'}], null, '\t');
// text is '[\n\t"e",\n\t{\n\t\t"pluribus": "unum"\n\t}\n]'

[Use mkwst_at_chromium.org plz., 2011-08-09 13:14:38]

On 2011/08/09 12:50:56, Pam wrote:

> Or, I see that "The optional space parameter [to stringify] produces a
> stringification of the value that is filled with line breaks and indentation
to
> make it easier to read."

It's changed a bit in the actual implementation, but WOW that's a much better
solution. :)

https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/JSON/str...

Thanks!

[kurrik.chromium, 2011-08-09 16:48:44]

LGTM.  I am fully behind this change.


On 2011/08/09 13:14:38, mkwst wrote:
> On 2011/08/09 12:50:56, Pam wrote:
> 
> > Or, I see that "The optional space parameter [to stringify] produces a
> > stringification of the value that is filled with line breaks and indentation
> to
> > make it easier to read."
> 
> It's changed a bit in the actual implementation, but WOW that's a much better
> solution. :)
> 
>
https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/JSON/str...
> 
> Thanks!

[Mike West, 2011-08-09 16:57:42]

On 2011/08/09 16:48:44, kurrik.chromium wrote:
> LGTM.  I am fully behind this change.

Cool.  As discussed, I'll land this tomorrow after you land your bit. :)

[Mike West, 2011-08-11 10:33:46]

On 2011/08/09 16:57:42, Mike West wrote:
> On 2011/08/09 16:48:44, kurrik.chromium wrote:
> > LGTM.  I am fully behind this change.
> 
> Cool.  As discussed, I'll land this tomorrow after you land your bit. :)

Rebasing this now; I'll land it once DumpRenderTree finishes building. *waiting*

[commit-bot: I haz the power, 2011-08-11 11:26:54]

Presubmit check for 7601024-5002 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit Warnings **
This change modifies the extension docs but the generated docs have not been
updated properly. See chrome/common/extensions/docs/README.txt for more info.
(Report problems to tessamac@chromium.org.)
 - Changes to chrome/common/extensions/docs/static/samples.html not reflected in
generated doc.
First build DumpRenderTree, then update the docs by running:
  chrome/common/extensions/docs/build/build.py

Presubmit checks took 1.0s to calculate.

[Mike West, 2011-08-11 11:29:53]

On 2011/08/11 11:26:54, I haz the power (commit-bot) wrote:

> ** Presubmit Warnings **
> This change modifies the extension docs but the generated docs have not been
> updated properly. See chrome/common/extensions/docs/README.txt for more info.
> (Report problems to mailto:tessamac@chromium.org.)

Weird.  I'll land this manually, and file a bug for Tessa. :)