globalizejs / globalize

A JavaScript library for internationalization and localization that leverages the official Unicode CLDR JSON data
https://globalizejs.com
MIT License
4.8k stars 605 forks source link

Docs: Code output placement #313

Closed rxaviers closed 9 years ago

rxaviers commented 10 years ago

Throughout the documentation, code output often follows a code for clarity. For example:

Globalize.locale( "en" );
Globalize.formatDate( new Date( 2010, 10, 30, 17, 55 ), { datetime: "short" } );
// "11/30/10, 5:55 PM"

Jörn once suggested that such comments to be placed above the code instead.

We need to evaluate and make the change consistent across the documentation.

scottgonzalez commented 10 years ago

Comments go above, though for documentation we sometimes put the comment on the same line. I don't think we ever put comments below. See http://learn.jquery.com/javascript-101/operators/ for an example.

agcolom commented 9 years ago

I'd be happy to help with that... I'm quite busy right now but I like doing this type of thing to relax... What sort of timeframe are you looking at and do you already have anyone interested in doing this or do you plan on producing a script to do this all automatically?

rxaviers commented 9 years ago

Hi Anne,

Having your help here would be wonderful. I have no one assigned to it yet, no automation script planned nor any urgency.

About the change itself... It seems like we have the options below.

(a) Placing the output above the commands:

Globalize.locale( "en" );

// "11/30/10, 5:55 PM"
Globalize.formatDate( new Date( 2010, 10, 30, 17, 55 ), { datetime: "short" } );

(b) Placing the output on the same line:

Too big lines:

Globalize.locale( "en" );
var output = Globalize.formatDate( new Date( 2010, 10, 30, 17, 55 ), { datetime: "short" } );
console.log( output ); // "11/30/10, 5:55 PM"

Normal-size lines:

Globalize.locale( "en" );
Globalize.formatDate( date, { datetime: "short" } ); // "11/30/10, 5:55 PM"

Any other ideas?

agcolom commented 9 years ago

@rxaviers Great. I'll work on this when I get a chance and will do it all in a branch on my fork and do a PR when ready...

rxaviers commented 9 years ago

Sounds great. Thanks

jzaefferer commented 9 years ago

@agcolom did you start this somewhere?

agcolom commented 9 years ago

@jzaefferer No not yet. I've not had any time yet.

sotojuan commented 9 years ago

Hi guys, going to help with this starting today!

rxaviers commented 9 years ago

:+1:

sotojuan commented 9 years ago

Hi, I have been working on this and made good progress, but I have a few questions:

I was told in the IRC that the rule is: Put the output on the side or the top if on the side is too long. Would "too long" mean longer than 80 characters? For example here the output messages ("Hello, Wolfgang...") look like they'd fit on the side, but that would make the line longer than 80 characters.

On the same note, when the function that produces the output is passed an object literal, how do you want me to format it?

For example, this:

formatter({
  first: "Wolfgang",
  middle: "Amadeus",
  last: "Mozart"
});
➡ "Hey, Wolfgang Amadeus Mozart"

Should the "Hey, Wolfgang Amadeus Mozart" be right next to the closing }); or at the top? Or where it is right now?

One last thing, on results like this one where it returns a big object, should I put that at the top or leave it under it?

Sorry for the many questions! I should be done with this by tonight after I recheck everything.

scottgonzalez commented 9 years ago

@jquery/content See above.

agcolom commented 9 years ago

@matachines lots of good questions here. For the 1st example, I would put the result either where it is or above in a single line comment. I'm still unsure which one would be best. If we leave it where it is, I think it would be nice to use a specific color to indicate that this is output. Anyone else has thoughts on this?

agcolom commented 9 years ago

@matachines sorry, ignore my color comment, we're in md!

sotojuan commented 9 years ago

@agcolom Thanks for the response. I personally would go for putting in the top since that is more consistent with the current rules, but I'll wait for confirmation.

One other thing I forgot to mention and noticed while working on the README.md is the use of the little arrows (➡). They're not mentioned in this issue and their use is inconsistent throughout the documentation, so I've been replacing them with a // because that's what this issue shows. If keeping them is better, let me know, although I think keeping them would make some of the above rules kinda weird (having an arrow next to the function makes sense, but having an arrow above the function is less intuitive than a comment in my opinion).

rxaviers commented 9 years ago

@agcolom, @jquery/content, is there any existing case where code output is placed as a comment above the code? I found one example at http://api.jquery.com/data/, where the code output is placed on the same line and it doesn't seem to care about length being bigger than 80 characters (although, it's important to note it didn't exceed 100).

On our style guide, we have "Single line comments go over the line they refer to". In my opinion, it's completely valid placing a comment before a code block. Because, it explains the code block. It's very analogous to a title explaining a text. But, I don't feel the same for code output. Because, code output is expected to be generated after the code is run, not prior to it.

The Mozilla Developer Network (MDN), uses // → after/below the code to demonstrate its output. For example:

var date = new Date(Date.UTC(2012, 11, 20, 3, 0, 0));

// toLocaleString without arguments depends on the implementation,
// the default locale, and the default time zone
console.log(new Intl.DateTimeFormat().format(date));
// → "12/19/2012" if run in en-US locale with time zone America/Los_Angeles

Ember.js also places code output after/below the code.

Ember.String.w("alpha beta gamma").forEach(function(key) {
  console.log(key);
});

// > alpha
// > beta
// > gamma

I'm wondering if we could treat code output the same way, i.e., after the code (in the same line or below it) since it seems more logic.

scottgonzalez commented 9 years ago

For docs, I think comments below are preferable, since they're showing results.

agcolom commented 9 years ago

Yes, that was my line of thoughts also (seems more logical). So let's go for after, on its own line.

agcolom commented 9 years ago

Still thinking about the arrow... Personally, I'm fine with using this to indicate output... but not strongly opinionated. So if anyone else feels strongly about one way or the other, I'd be happy to hear. What we do here will lead the way to other places if we ever need to display output results elsewhere.

rxaviers commented 9 years ago

Ok. For clarity. Please, correct me I'm wrong. But, I understood we're going to demonstrate a code output with a comment after the code by placing it:

  1. on its own line if shorter than 100 characters (considering tabs as 4 spaces). Or:
  2. below the line otherwise.
timmywil commented 9 years ago

I like always after with an arrow.

rxaviers commented 9 years ago

About the arrow... Globalize uses , MDN , Ember >. Does anyone have a preference for the symbol? I'm ok with either the one that Globalize is currently using or the one that MDN uses.

sotojuan commented 9 years ago

OK guys, thanks a lot for the suggestions and discussion. I'm probably going with @rxaviers just said above, with an arrow because it's a very intuitive way to show output. Going to fix everything up right now and report back in a bit.

scottgonzalez commented 9 years ago

I prefer the simplicity of >. We can always add a build step that converts // > to whatever we want if we decide to use a fancy arrow.

rxaviers commented 9 years ago

Thanks for the input so far everyone. One more question. When placing the output comment below the code, should it be preceded by a blank line (as style guide suggests)? I'm assuming not (it doesn't make sense in this case). But, just let me know if anyone things we should.

agcolom commented 9 years ago

I'm thinking no blank line before.

rxaviers commented 9 years ago

:+1:

agcolom commented 9 years ago

I like Scott's idea of // > (sorry if this series of keys don't work here, I'm on my phone!) and change it to whatever we want on build, unless we have that sequence elsewhere.. (Sorry I can't check right now)

rxaviers commented 9 years ago

So, > it is.

@agcolom, should we summarize what we've agreed on in this discussion in the style guides or anywhere else?

agcolom commented 9 years ago

Definitely!

scottgonzalez commented 9 years ago

A summary can go into https://github.com/jquery/contribute.jquery.org/issues/75 :-)

sotojuan commented 9 years ago

OK guys, so I just went through the docs in my fork and formatted it according to these rules (using // >):

I also aligned code outputs in the same code block. The fork branch is here, let me know if I missed anything or if the rules change.

Edit: I can never get Markdown links right.

rxaviers commented 9 years ago

Awesome @matachines. Please, go ahead and create a PR https://github.com/jquery/globalize/compare/master...matachines:313-code-output-placement.

scottgonzalez commented 9 years ago

From the discussion above, I thought we were always placing the comments on the following line, never on the same line.

agcolom commented 9 years ago

+1 to having // > on its own line also (seems to be what we agreed above)

rxaviers commented 9 years ago

We have a clear consensus about having the code output after in its own line. Although, I understood we were also ok with the inline comment (i.e., after in the same line), which I think is handy in cases like the below.

.numberFormatter()( pi );                                // > "3.142"
.numberFormatter({ maximumFractionDigits: 5 })( pi );    // > "3.14159"
.numberFormatter({ round: "floor" })( pi );              // > "3.141"
.numberFormatter({ minimumFractionDigits: 2 })( 10000 ); // > "10,000.00"
.numberFormatter({ style: "percent" })( 0.5 );           // > "50%"

I feel bad for the miscommunication here and having @matachines re-re-work his PR, which I tried to avoid with https://github.com/jquery/globalize/issues/313#issuecomment-78132666.

scottgonzalez commented 9 years ago

I feel bad for the miscommunication here and having @matachines re-re-work his PR, which I tried to avoid with #313 (comment).

To be fair, the next comment was simply:

I like always after with an arrow.

And then PR was pretty much instant, so there wasn't time for a full discussion.

sotojuan commented 9 years ago

No problem, just let me know when you guys decide on a style. Changing stuff around only takes a few minutes.

rxaviers commented 9 years ago

Great, so let's wait to hear what everyone thinks of the case of https://github.com/jquery/globalize/issues/313#issuecomment-78315661 using inline comment.

EDIT: Along with your vote, please provide an example if different than inline.

rxaviers commented 9 years ago

@agcolom @timmywil @scottgonzalez @jquery/content please when you find time, chime in :top: with your vote.

agcolom commented 9 years ago

@rxaviers do you have a big block like this anywhere and regularly, or usually it is just one output at a time?

scottgonzalez commented 9 years ago

I prefer separate lines. The horizontal spacing can make it hard to pair the code with the output. I have a feeling that pairing is even harder for dyslexics and low vision users.

rxaviers commented 9 years ago

Yeap, we do on https://github.com/matachines/globalize/blob/313-code-output-placement/README.md

agcolom commented 9 years ago

How about on its own line with a blank line after? globalize

scottgonzalez commented 9 years ago

In my mind, that's actually how it would look. I guess we're just inverting the normal rules at this point :-P

agcolom commented 9 years ago

ha ha, yes ;-) but I agree also that this is how it should be.

rxaviers commented 9 years ago

I'm fine with it. Ok, then. @matachines, you are free to go. Could you please update your PR accordingly?

sotojuan commented 9 years ago

Done! Let me know if I missed anything.

timmywil commented 9 years ago

My vote hasn't changed. I was imagining what @agcolom put in her recent example.

rxaviers commented 9 years ago

Thanks