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

Line break before @returns #782

Closed
thernstig opened this issue Sep 6, 2021 · 21 comments · Fixed by #1017
Closed

Line break before @returns #782

thernstig opened this issue Sep 6, 2021 · 21 comments · Fixed by #1017

Comments

@thernstig
Copy link

thernstig commented Sep 6, 2021

Motivation

Line breaks matters for readability, especially having a consistent line break style in documentation. I want to be able to enforce line breaks before @returns statements.

Current behavior

There is no configuration for it.

Desired behavior

I want this to be invalid:

  /**
   * Do something.
   *
   * @param {string} fooA - Some text
   * @returns {string} fooB - Some text
   */

I want this to be valid (with the new rule)

  /**
   * Do something.
   *
   * @param {string} fooA - Some text
   *
   * @returns {string} fooB - Some text
   */

Or for those who prefer, the other way around (the first example valid, the second invalid).

@brettz9

This comment was marked as outdated.

@brettz9 brettz9 closed this as completed Sep 6, 2021
@thernstig

This comment was marked as outdated.

@brettz9 brettz9 reopened this Sep 13, 2021
@brettz9
Copy link
Collaborator

brettz9 commented Sep 13, 2021

Apologies about that.

The rule doesn't allow enforcing a specific number of lines before a given tag, but if @returns is your last item and you also want lines between @param, you could do this:

`jsdoc/tag-lines`: ['warn', 'always', {
  noEndLines": true
}]

However, if you don't want lines between @param, and just one empty line before @returns, then it seems like we need new configuration (though I don't think it would be trivial to add support for it given the current rule design).

@thernstig
Copy link
Author

I want just one before @returns. So a new config option would be nice. Or alternatively that "noEndLines": true, actually means no end lines no matter the other config.

To me, this looks nicer:

  /**
   * Do something.
   *
   * @param {string} fooA - Some text
   * @param {string} fooB - Some text
   * @param {string} fooC - Some text
   *
   * @returns {string} fooD - Some text
   */

than this:

  /**
   * Do something.
   *
   * @param {string} fooA - Some text
   *
   * @param {string} fooB - Some text
   *
   * @param {string} fooC - Some text
   *
   * @returns {string} fooD - Some text
   */

or

  /**
   * Do something.
   *
   * @param {string} fooA - Some text
   * @param {string} fooB - Some text
   * @param {string} fooC - Some text
   * @returns {string} fooC - Some text
   */

@Ulrikop
Copy link

Ulrikop commented Oct 9, 2021

I stumbled across this issue and have to say: It would be great if I can configure jsdoc/tag-lines between tags of same group + between different groups.

The example of @thernstig is great but + blank lines between same tags of an other group like @example

  /**
   * Do something.
   *
   * @param {string} fooA - Some text
   * @param {string} fooB - Some text
   * @param {string} fooC - Some text
   *
   * @returns {string} fooD - Some text
   *
   * @example
   * ```
   * myFunction('a', 'b', 'c');
   * ```
   *
   * @example
   * ```
   * myFunction('z', 'y', 'z');
   *```
   */

I tried something like 'jsdoc/tag-lines': ['error', 'always', { tags: { param: { lines: 'never' } }, noEndLines: true }] but it didn't produce the expected behavior in all scenarios, so I had to disable the rule.

@thernstig

This comment was marked as outdated.

@jaydenseric
Copy link
Contributor

I (and a lot of other people actually) have come to the conclusion that all formatting should be done by formatters (e.g. Prettier, Deno fmt), and not linters that visually report errors in the editor.

There are Prettier plugins for formatting JSDoc. If they are not to your taste they can be improved or alternatives can be published.

IMO eslint-plugin-jsdoc should remove all formatting related rules and stick to rules that relate to the semantics of JSDoc tags and content.

@thernstig
Copy link
Author

thernstig commented Oct 28, 2021

@jaydenseric I can agree to that, yet that is a separate issue still. Until it is done, I would at least like this one to be solved.

@thernstig
Copy link
Author

@brettz9 this would be awesome if you can fix this, third most upvoted issue now 😄

@brettz9
Copy link
Collaborator

brettz9 commented Nov 10, 2022

Still no promises of implementing, but I think one key obstacle here is determining how this behavior will work with or be added to the existing tag-lines behavior.

I think there must be cases where users may wish to have tag-lines behavior to ensure lines (or no lines) after tags (though optionally collapsing for final tags), along with the proposed behavior of a line break before certain tags.

I also think it makes sense to have a more generic solution allowing line breaks before certain tags (by default, @returns makes sense), and if so, whether to enforce line breaks before each of these tags or only the first tag in the sequence (e.g., before a series of @param).

@thernstig
Copy link
Author

@brettz9 I tried to find tag-lines in the TOC at https://github.com/gajus/eslint-plugin-jsdoc by clicking the link, but it does not go to any location, so it seems to be missing (same for text-escaping and valid-types).

I agree with your reasoning. Having a generic solution to define which tags to have line breaks before/after would be the best, maybe with a set of defaults for certain tags such as @returns having one before.

@brettz9
Copy link
Collaborator

brettz9 commented Nov 11, 2022

Regarding the missing rules, ah, yes, they are still being truncating per #886 . You can view the rule with the source doc at https://github.com/gajus/eslint-plugin-jsdoc/blob/master/.README/rules/tag-lines.md and the test examples at https://github.com/gajus/eslint-plugin-jsdoc/blob/master/test/rules/assertions/tagLines.js

@thernstig
Copy link
Author

@brettz9 I just tried the sort-tags rule and the --fix option and was not happy with the results due to the line-breaks between tags.

I checked https://github.com/gajus/eslint-plugin-jsdoc/blob/main/src/defaultTagOrder.js and would probably want the the feature worked nicely with all those tags, adding line breaks between "blocks" of those for how they are sorted with sort-tags.

At least I know I definitely would want a line break both before and after @returns and line breaks before/after all @params.

@thernstig
Copy link
Author

thernstig commented Feb 14, 2023

@brettz9 I thought about this a bit more, and it is possible that there should be a more holistic approach for how to achieve this. But that would require bigger changes probably in the already existing configurations, meaning a breaking change. I bring it up at least to start the discussion.

If we check https://developer.wordpress.org/coding-standards/inline-documentation-standards/javascript/#functions it shows:

/**
 * Summary. (use period)
 *
 * Description. (use period)
 *
 * @since      x.x.x
 * @deprecated x.x.x Use new_function_name() instead.
 * @access     private
 *
 * @class
 * @augments parent
 * @mixes    mixin
 *
 * @alias    realName
 * @memberof namespace
 *
 * @see  Function/class relied on
 * @link URL
 * @global
 *
 * @fires   eventName
 * @fires   className#eventName
 * @listens event:eventName
 * @listens className~event:eventName
 *
 * @param {type}   var           Description.
 * @param {type}   [var]         Description of optional variable.
 * @param {type}   [var=default] Description of optional variable with default variable.
 * @param {Object} objectVar     Description.
 * @param {type}   objectVar.key Description of a key in the objectVar parameter.
 *
 * @yield {type} Yielded value description.
 *
 * @return {type} Return value description.
 */

My preference would be to allow me to use something like sort-tags but with more freedom on how to sort them, including line breaks between tag groups

For example, here is some psuedo-code (not including all parts shown above):

    "jsdoc/sort-tags": [
      "warn",
      "always",
      {
        "tagGroups": [
          {
            "tags": ["@since", "@deprecated", "@access"],
            "linesBefore": 1,
            "linesAfter": 1
          },
          {
            "tags": ["@param"],
            "linesBefore": 1,
            "linesAfter": 1
          },
          {
            "tags": ["@yield"],
            "linesBefore": 1,
            "linesAfter": 1
          },
          {
            "tags": ["@returns"],
            "linesBefore": 1,
            "linesAfter": 1
          }
        ],
        "mergeLinesAfterAndBefore": 1,
        "linesAfterLastGroup": 0
      }
    ],

What I want to achieve is to:

  • Specify how tags should be but in "groups"
  • The spacing between broups
  • If one group has "linesBefore": 1, and a preceding group has "linesAfter": 1,, I can set "mergeLinesAfterAndBefore": 1 so that they "merge" and only get 1 total line breaks between instead of 2. If I set "mergeLinesAfterAndBefore": 2 each group gets 2 lines between them.
  • If I set "linesAfterLastGroup": 0 the last group, no matter which it is, always get 0 line breaks as I do not want a line break just before the end of the JSDoc group.
  • One more option I did not add above, might be what to do with tags that are not in the list. Maybe an option "unspecificedTagsPlacement: "last/first". Or maybe even better to add a object { "tags": ["REST"],} so that one can add a tag group for the "REST" anywhere wanted.

I have no idea how maintainable, configurable and similar this approach would be. But it perfectly highlights what I would want to achieve and I guess many others would want. For example https://developer.wordpress.org/coding-standards/inline-documentation-standards/javascript/#functions where the above sample code was stolen from.

@brettz9
Copy link
Collaborator

brettz9 commented Feb 15, 2023

It looks like a fine approach, but I don't expect I'll be able to work on this.

@thernstig
Copy link
Author

@brettz9 I understand. You do so much for the echo system already so no one can expect anything more of you, you have given back more than what most do in their life time in open source ❤️

I wish I was the one to implement this but my time is constrained as well right now.

@brettz9
Copy link
Collaborator

brettz9 commented Apr 9, 2023

While I normally appreciate more granular control, do you see a use case where someone would want different spacing for before and after tag groups and not just a merged total between tag groups?

@thernstig
Copy link
Author

thernstig commented Apr 10, 2023

@brettz9 not really, no. Maybe it is better to leave that out and leave for more granular control later if requested. I would assume users want 1 empty line between tag groups.

In the case they want no empty line, the intention is that they should place it in the same tag group. The array order in the tag group should of course matter for sorting within the tag group.

edit

Then most of the other options can go away as well to start with. The only one I'd wish to keep would be { "tags": ["REST"],} so that one can place the "REST" (the tags not specified) somewhere else. They should be sorted in alphabetical order automatically.

Then I can chose to place them in a specific group at any place, or in its own group e.g. last or first which might be what users want.

@brettz9
Copy link
Collaborator

brettz9 commented Apr 12, 2023

For those who want lines either before the first group or after the last group—since both of these may be of interest to those not using the rule—I think we should add that functionality in another rule. So it should only be lines between groups in this rule.

By the way, we already have an "-other" special tag, so I think we can use that instead of "REST". However, rather than sorting the rest alphabetically, it keeps them as is. Since check-tags can ensure only known tags are used, I don't think it is too big of a burden to expect users to alphabetize all viable tags themselves here.

brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue Apr 12, 2023
@brettz9
Copy link
Collaborator

brettz9 commented Apr 12, 2023

If you find some time to kick the tires on #1017 , @thernstig , I did find some energy for this. Again, I think we should have a separate rule to add or remove lines before all tags or after all tags, but I believe it should work for lines between tag groups now.

brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue Apr 17, 2023
brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue Apr 18, 2023
… and after) them and removal of whitespace within them; fixes gajus#782
brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue Apr 18, 2023
… and after) them and removal of whitespace within them; fixes gajus#782

BREAKING CHANGE:

`tagSequence` array of string tags argument must be specified instead as array of `{tags: []}` objects, with each object representing groups which can gain optional whitespace between them; if none are needed, just put all tags into the array of a single `{tags: []}`
brettz9 added a commit that referenced this issue Apr 18, 2023
… and after) them and removal of whitespace within them; fixes #782

BREAKING CHANGE:

`tagSequence` array of string tags argument must be specified instead as array of `{tags: []}` objects, with each object representing groups which can gain optional whitespace between them; if none are needed, just put all tags into the array of a single `{tags: []}`
@thernstig
Copy link
Author

Amazing work @brettz9, you're a hero 💯

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.

4 participants