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

Examples indented by 2 spaces #46

Closed
RunDevelopment opened this issue Feb 4, 2021 · 20 comments
Closed

Examples indented by 2 spaces #46

RunDevelopment opened this issue Feb 4, 2021 · 20 comments

Comments

@RunDevelopment
Copy link
Contributor

Right now, the code in @example tags is indented by 2 spaces. Why is there an indentation at all (this seems to be uncommon) and why 2 spaces specifically?

@hosseinmd
Copy link
Owner

That is standard of Google style guide adding indented by 4 spaces, we changed that to 2 spaces. The reason of adding indented is for readability.

@RunDevelopment
Copy link
Contributor Author

That is standard of Google style guide

Which one? The Google JS style guide doesn't mention @example tags.

The reason for adding indented is for readability.

I would argue that the spaces wouldn't be necessary if the @example tag was the last tag. It's very common that the @example tag is the last one in a comment. I always wondered by this plugin places this tag so high up.

As an example, let's look at lodash's doc (I just picked a file at random). They don't indent code, always have @example last, and use an empty line to separate the example code from the rest of the comment. Very clear and readable.

@hosseinmd
Copy link
Owner

hosseinmd commented Feb 4, 2021

I don't agree but if anyone (contributors or others) agrees with you, I will accept.
@mrvisser @forivall @aqzhyi @danielpza @reggev @majidbigdeli

@mrvisser
Copy link
Contributor

mrvisser commented Feb 4, 2021

@RunDevelopment the indentation is not specific to the @example tags, it's general guidance for line wraps for any tag:

https://google.github.io/styleguide/javascriptguide.xml?showone=Comments#Comments (search "JSDoc Indentation")

I'm not 100% sure if the line-wrapping is handled consistently across non-@example tags in this library though?

I think it's a bit of an opinion, which is fine by me for an automated formatters, particularly when there is precedent. The key thing for me is that it is consistent and it doesn't suddenly change and start causing lots of phantom changes in my code-base when I upgrade.

@RunDevelopment
Copy link
Contributor Author

Then may I suggest adding an option to turn the indentation off?

I know that this goes against Prettier's philosophy of being "opinionated" but it might make this plugin more fitting for others and myself.


Also, the 2 spaces should probably be changed to tabWidth.

@hosseinmd
Copy link
Owner

Then may I suggest adding an option to turn the indentation off?

I know that this goes against Prettier's philosophy of being "opinionated" but it might make this plugin more fitting for others and myself.

Also, the 2 spaces should probably be changed to tabWidth.

This could be done.

@mrvisser
Copy link
Contributor

mrvisser commented Feb 4, 2021

@RunDevelopment can you show any examples of what you're trying to write up in an @example block where the indentation is causing a tangible issue that isn't founded on a counter-opinion of indentation?

@RunDevelopment
Copy link
Contributor Author

tangible issue that isn't founded on a counter-opinion of indentation

Not really, it's just indentation after all. I wouldn't be able to find any examples of "tangible issue" in a discussion of tabs vs spaces or tabWidth 2 vs 4 vs 8 either.

The reason I opened this issue is that the 2 spaces seemed arbitrary to me (as in why 2 spaces and why spaces at all). I hadn't seen JSDoc comments with this style before and big projects like lodash also don't use it.

@mrvisser
Copy link
Contributor

mrvisser commented Feb 4, 2021

Cool, thanks. My question wasn't really designed to be dismissive but more thinking if there are issues with code representation in cases where maybe <pre></pre>-style content was necessary, so thought maybe there could be more direct solutions to pre-formatted text scenarios.

Based on this conversation I'd personally be in favour of fixing it to tabWidth, but opposed to making it configurable if I were core maintainer.

@RunDevelopment
Copy link
Contributor Author

RunDevelopment commented Feb 4, 2021

if there are issues with code representation in cases where maybe <pre></pre>-style content was necessary

I had thought about it in that way but yes, there is an issue. I just tested it with JSDoc and it doesn't remove the indentation, so all examples have extra indentation. (EXCEPT for the first line. This actually a property of the comment parser JSDoc and we use. It trims the example code.)

Input code:
image

Output:
image

@mrvisser
Copy link
Contributor

mrvisser commented Feb 4, 2021

Yea, that's problematic, and trimming like that might be common among parsers.

Does JSDoc have a spec around what tags it's treating as pre-formatted text by default v.s. not? We might just need to have similar support for those defaults.

@RunDevelopment
Copy link
Contributor Author

Does JSDoc have a spec around what tags it's treating as pre-formatted text by default v.s. not?

FTFY. I can't tell you because the docs don't say much about @example. Is there even a specification for JSDoc? Wikipedia claims there is but doesn't provide any sources or a link to it.

@mrvisser
Copy link
Contributor

mrvisser commented Feb 4, 2021

The link you posted has documentation saying that the content of @example is treated specially, specifically as highlighted code / pre-formatted text:

The text that follows this tag will be displayed as highlighted code.

So it probably makes sense to have some exceptions to the indentation for specialized tag definitions, as opposed to unstructured that can be soft-wrapped and be hard to discern from its peers such as @param, which I think is the in-editor readability wart @hosseinmd was referencing.

We can then do further improvements that way such as avoiding soft-wrapping entirely for tags like @example, because that will also break code examples that are intended to be treated as <pre></pre>, etc...

@RunDevelopment
Copy link
Contributor Author

So, what will the final answer be? ^^

@hosseinmd said, "This could be done.", but I don't know whether he was referring to "adding an option" or "2 spaces should probably be changed to tabWidth" or both. Could you please clarify @hosseinmd?

avoiding soft-wrapping entirely for tags like @example

That's already the case right now.

@mrvisser
Copy link
Contributor

mrvisser commented Feb 4, 2021

In my mind I had:

  1. Defer to tabWidth for indentation level
  2. Add a collection of tag names whose content are treated implicitly as pre-formatted text, thus not modified for soft-wrapping (I guess this doesn't happen, wasn't sure), but also for indentation. @example is in there, maybe there are others.

@hosseinmd
Copy link
Owner

@hosseinmd said, "This could be done.", but I don't know whether he was referring to "adding an option" or "2 spaces should probably be changed to tabWidth" or both. Could you please clarify @hosseinmd?

I think both is ok. we could implement both.

  1. Add a collection of tag names whose content are treated implicitly as pre-formatted text, thus not modified for soft-wrapping (I guess this doesn't happen, wasn't sure), but also for indentation. @example is in there, maybe there are others.

I don't understand exactly

@hosseinmd
Copy link
Owner

  1. Defer to tabWidth for indentation level

Why?

@mrvisser
Copy link
Contributor

mrvisser commented Feb 4, 2021

I don't understand exactly

Because some tags are treated implicitly as preformatted text. So the indentation is not stripped, it actually becomes part of the output which is not the desired effect of stylistic indent.

Why?

Because it's consistent with how users have configured prettier to indent their code.

@hosseinmd
Copy link
Owner

Because it's consistent with how users have configured prettier to indent their code.

IMO it is a benefit

@hosseinmd
Copy link
Owner

There is two stuff.

  1. Defer to tabWidth for indentation level

Done.

2. Add a collection of tag names whose content are treated implicitly as pre-formatted text, thus not modified for soft-wrapping (I guess this doesn't happen, wasn't sure), but also for indentation. @example is in there, maybe there are others.

Not solved yet

hosseinmd added a commit that referenced this issue Mar 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants