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

Clean up key-spacing documentation #9900

Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion documentation Relates to ESLint's documentation good first issue Good for people who haven't worked on ESLint before help wanted The team would welcome a contribution from the community for this issue

Comments

@not-an-aardvark
Copy link
Member

As of ESLint v4.16.0, the key-spacing documentation is difficult to understand. All of the options are presented in a single list in the Options section, but there are actually multiple categories of options, and some options can be nested under other options, and it's not clear what is allowed from reading the top of the file. I think we should reorganize the documentation to more clearly describe what configurations are allowed without relying so much on examples.

@not-an-aardvark not-an-aardvark added documentation Relates to ESLint's documentation accepted There is consensus among the team that this change meets the criteria for inclusion labels Jan 26, 2018
@alberto alberto added good first issue Good for people who haven't worked on ESLint before help wanted The team would welcome a contribution from the community for this issue labels Jan 28, 2018
@abiduzz420
Copy link
Contributor

abiduzz420 commented Jan 29, 2018

Let me look into the issue. I shall take it up.

EDIT: Did not get to work on it from past few days, on the task right now

@platinumazure
Copy link
Member

Thanks @abiduzz420! Let us know if we can help.

@abiduzz420
Copy link
Contributor

What do you guys think about it: https://gist.github.com/abiduzz420/75d6f8b8508eb3d9fed8137c2ccefc20
Open to changes.

@platinumazure
Copy link
Member

@abiduzz420 Looks promising! Go ahead and open a PR and we can take a closer look.

@abiduzz420
Copy link
Contributor

abiduzz420 commented Feb 9, 2018

One of the problems I faced while reading the examples is they're not properly highlighted. Meaning I need to clearly read whether it's the correct code or incorrect code example. May be we could have incorrect code examples in warning boxes (bootstrap way) and show correct code snippets in green.
Another alternative of showing code examples could be showing it side by side (incorrect and the correct code)

@platinumazure
Copy link
Member

platinumazure commented Feb 9, 2018

Strange, we are supposed to have code that highlights examples based on the immediately preceding text (to wit, Examples of **incorrect** code vs Examples of **correct** code). Let me dig into that on the side...

EDIT: Never mind, our code just adds an icon (thumbs-up or thumbs-down) before the paragraph in question. I agree it would be nice to style the examples' <pre> blocks a little better. Would you like to open an issue on our website repo? Thanks!

@abiduzz420
Copy link
Contributor

Yeah sure I will do it.

@platinumazure
Copy link
Member

platinumazure commented Feb 9, 2018

@abiduzz420 Actually I decided to try to see if I could solve the example styling problem and I came up with something simple pretty quickly. So no need to create an issue. Please feel free to follow eslint/archive-website#453 if you want to track the progress of that enhancement 😄

Also, here's how the key-spacing page (without your changes) would look with the style change, so feel free to comment on the link above if you think it could be improved. https://5a7dce1f7b6ee879dfded597--eslint.netlify.com/docs/rules/key-spacing

@abiduzz420
Copy link
Contributor

I'll look into it in the evening. Differentiating incorrect code snippet from correct code would help.

@platinumazure
Copy link
Member

@abiduzz420 I did some preliminary digging and documented my findings here: eslint/archive-website#454

This was referenced Mar 19, 2018
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Aug 10, 2018
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Aug 10, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion documentation Relates to ESLint's documentation good first issue Good for people who haven't worked on ESLint before help wanted The team would welcome a contribution from the community for this issue
Projects
None yet
4 participants