Update JS documentation to follow Doxygen format (remove curly braces)

The '$' character in a variable name is a convention used to distinguish a jQuery set of DOM elements for a DOM element. It's a hint that jQuery methods an be used with this variable.

I think so, in case it's not it should be added in this issue: #1778828-7: [policy, no patch] Update JS coding standards

Should we also remove the curly braces from other JS in core? :)

Other locations: toolbar.module, contextual.module (a little bit), ckeditor.module (a tiny bit), dropbutton, matchmedia, drupal.js, displace.js, debounce.js.

In the background of this issue we can find #1337022: [policy, no patch] Create/adopt JavaScript docs standards compatible with a JS documentation parser and #25901: Parse/save/display JavaScript files.

Re: #5 ... Yes.

I have created a patch with the js documentation cleaning of the files in core. I guess I didn't forget anyone.

Also I hope the patch is right (it's my first patch)

Feedback is welcome.

rteijeiro thank you for the patch. It seems like you may have gotten more changes in your diff than just comments. Try rebasing your branch on the current 8.x branch, so that just the changes you made are present and not any changes from when you created your branch to the time that you created the diff. Does that makes sense?

Hi Jesse, sorry for the mistake. I fetched and rebased with 8.x branch. Hope the patch is now right. If not, just notify me again ;)

Sorry, this patch is crappy again. I will try to fix it.

I guess now I did the diff with the correct branch :)

+++ b/core/misc/displace.jsundefined
@@ -34,11 +34,11 @@
-   * @param {boolean} broadcast
+   * @param boolean broadcast

Should be uppercased.

+++ b/core/misc/displace.jsundefined
@@ -34,11 +34,11 @@
-   * @return {object}
+   * @return object

Idem.

+++ b/core/misc/displace.jsundefined
@@ -54,7 +54,7 @@
-   * @return {object}
+   * @return object

Idem.

(And several more of these. I checked with nod_ to make sure that's the goal.)

+++ b/core/modules/toolbar/js/toolbar.menu.jsundefined
@@ -96,10 +95,9 @@ var activeItem = drupalSettings.basePath + drupalSettings.currentPath;
+     * @param Integer level

"Integer" does not exist in JavaScript. This should be "Number".

Also did the following changes:

- Uppercased 'string' to 'String'.
- Uppercased 'number' to 'Number'.
- Changed 'bool' by 'Boolean'.

Awesome! I wanted to RTBC, but there's just one more thing…

+++ b/core/misc/ui/external/qunit.jsundefined
@@ -40,8 +40,9 @@ var QUnit,
+	 * @param String|Error error
+   *
+	 * @return String error message

We shouldn't be changing external libraries.

It's true. I didn't realized about that :)

Fixed.

Awesome, thanks! :)

Great!! My first patch ^_^

Thanks, guys!!

Looks great, thank you so much rteijeiro!!!!

Needs a re-roll

error: patch failed: core/misc/drupal.js:283
error: core/misc/drupal.js: patch does not apply

Rerolled.

Please, tell me if everything is ok.

Rerolled with the latest changes in 8.x branch.

#27: update-js-documentation-1958246-27.patch queued for re-testing.

#27: update-js-documentation-1958246-27.patch queued for re-testing.

Great command, @Cottser, thanks!

Updated the patch according the #34 comment.

Forgot needs review status :(

A grep through all the core js shows all curly braces are accounted for by update-js-documentation-1958246-35.patch in comment #35.

#35: update-js-documentation-1958246-35.patch queued for re-testing.

Thanks Cottser :)

Hope it will be merged before someone changes one of those files :P

This patch makes jsdoc useless btw. Not sure how important that is but it's too bad.

What abou this? #1337022: [policy, no patch] Create/adopt JavaScript docs standards compatible with a JS documentation parser

yeah don't get me started on that one.

The thing is that with the default jsdoc template, without the {} it will think the type is actually the name of the variable and we end up with documentation like annonce(String, String) instead of annonce(text, priority).

Also if we want to rely on a documentation tool and not having to write our own, we have to go with JSDoc. If it's good enough for google, it's good enough for us: http://google-styleguide.googlecode.com/svn/trunk/javascriptguide.xml?sh...

If someone wants to write a doxygen parser for JS sure, we can go with that. Won't be me though and I'd like JS docs in the near future, either as jsdoc output or on api.d.o, whichever comes first.

Sounds like this needs more discussion?

Definitely. If Doxygen isn't an option for javascript it probably doesn't make sense to comment in its style.

If that is true about Doxygen then we'd be better off going with the best available JS documenter and writing a Drupal standards filter for it if necessary.

One of the key questions is whether we could integrate its output with the PHP file documentation.

@cilefen that has been worked on by attiks: #1337022-80: [policy, no patch] Create/adopt JavaScript docs standards compatible with a JS documentation parser and his proof of concept is still online: http://drupalapi.attiks.com/api/js-samples/files/7 since JS is so different than PHP there are a few concepts that can't be properly handled by the api module.

Add link to JS coding standards

patch failes..

error: patch failed: core/modules/ckeditor/js/ckeditor.admin.js:249
error: core/modules/ckeditor/js/ckeditor.admin.js: patch does not apply
error: core/modules/contextual/contextual.toolbar.js: No such file or directory
error: core/modules/overlay/overlay-parent.js: No such file or directory
error: patch failed: core/modules/toolbar/js/toolbar.js:168
error: core/modules/toolbar/js/toolbar.js: patch does not apply
error: patch failed: core/modules/toolbar/js/toolbar.menu.js:68
error: core/modules/toolbar/js/toolbar.menu.js: patch does not apply

About this issue: No.

Patch to come in another issue. I'll link to it once patch is up.