Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Docs: Code output placement #313

Closed
rxaviers opened this issue Sep 23, 2014 · 49 comments
Closed

Docs: Code output placement #313

rxaviers opened this issue Sep 23, 2014 · 49 comments

Comments

@rxaviers
Copy link
Member

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
Copy link
Contributor

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.

@rxaviers rxaviers changed the title Docs: Code output comment placement Docs: Code output placement Sep 23, 2014
@agcolom
Copy link
Contributor

agcolom commented Oct 3, 2014

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
Copy link
Member Author

rxaviers commented Oct 3, 2014

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
Copy link
Contributor

agcolom commented Oct 3, 2014

@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
Copy link
Member Author

rxaviers commented Oct 3, 2014

Sounds great. Thanks

@jzaefferer
Copy link
Contributor

@agcolom did you start this somewhere?

@agcolom
Copy link
Contributor

agcolom commented Dec 12, 2014

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

@sotojuan
Copy link
Contributor

sotojuan commented Mar 9, 2015

Hi guys, going to help with this starting today!

@rxaviers
Copy link
Member Author

rxaviers commented Mar 9, 2015

👍

@sotojuan
Copy link
Contributor

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
Copy link
Contributor

@jquery/content See above.

@agcolom
Copy link
Contributor

agcolom commented Mar 10, 2015

@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
Copy link
Contributor

agcolom commented Mar 10, 2015

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

@sotojuan
Copy link
Contributor

@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
Copy link
Member Author

@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
Copy link
Contributor

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

@agcolom
Copy link
Contributor

agcolom commented Mar 10, 2015

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

@agcolom
Copy link
Contributor

agcolom commented Mar 10, 2015

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
Copy link
Member Author

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
Copy link

I like always after with an arrow.

@rxaviers
Copy link
Member Author

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Member Author

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
Copy link
Contributor

agcolom commented Mar 10, 2015

I'm thinking no blank line before.

@rxaviers
Copy link
Member Author

👍

@agcolom
Copy link
Contributor

agcolom commented Mar 10, 2015

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
Copy link
Member Author

So, > it is.

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

@agcolom
Copy link
Contributor

agcolom commented Mar 10, 2015

Definitely!

@scottgonzalez
Copy link
Contributor

A summary can go into jquery/contribute.jquery.org#75 :-)

@sotojuan
Copy link
Contributor

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

  • If code output fits in a line under 100 characters, put it on the side
  • If not, under it with no blank line

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
Copy link
Member Author

Awesome @matachines. Please, go ahead and create a PR master...matachines:313-code-output-placement.

@scottgonzalez
Copy link
Contributor

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

@agcolom
Copy link
Contributor

agcolom commented Mar 11, 2015

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

@rxaviers
Copy link
Member Author

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 #313 (comment).

@scottgonzalez
Copy link
Contributor

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
Copy link
Contributor

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

@rxaviers
Copy link
Member Author

Great, so let's wait to hear what everyone thinks of the case of #313 (comment) using inline comment.

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

@rxaviers
Copy link
Member Author

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

@agcolom
Copy link
Contributor

agcolom commented Mar 12, 2015

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

@scottgonzalez
Copy link
Contributor

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
Copy link
Member Author

@agcolom
Copy link
Contributor

agcolom commented Mar 12, 2015

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

@scottgonzalez
Copy link
Contributor

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

@agcolom
Copy link
Contributor

agcolom commented Mar 12, 2015

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

@rxaviers
Copy link
Member Author

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

@sotojuan
Copy link
Contributor

Done! Let me know if I missed anything.

@timmywil
Copy link

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

@rxaviers
Copy link
Member Author

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants