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

Setup .editorconfig #1082

Merged
merged 2 commits into from
Mar 2, 2018
Merged

Setup .editorconfig #1082

merged 2 commits into from
Mar 2, 2018

Conversation

joshbruce
Copy link
Member

@joshbruce joshbruce commented Feb 26, 2018

Marked version: 0.3.16

Proposal type: project operations

What pain point are you perceiving?

Tabs are rendered as literal tab characters, not spaces using Sublime Text 3.

Was working on #1076 and noticed the code samples were rendering funny when it came to whitespace and text wrapping. Realized it was because my editor was using literal tabs instead of spaces.

What solution are you suggesting?

A .editorconfig file following similar rules as lint options...specifically, spaces not tabs.

See also http://editorconfig.org

Could be accepted before or after #1076

@joshbruce joshbruce added the RR - refactor & re-engineer Results in an improvement to developers using Marked, or end-users, or both. label Feb 26, 2018
Copy link
Member

@UziTech UziTech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally use tabs for indentation on my projects so different developers can use any number of character widths that they want for indentation. I worked with a guy in college who was almost blind and preferred 8 character indentation. Thankfully we used tabs on all of our projects so I didn't need to stare at all of that whitespace in my editor.

I prefer 2 character width indentation and I work on quite a few projects that use spaces so it really doesn't matter to me. (I use the tab key for indentation anyway and let my editor figure out if it should put spaces or tabs in)

@joshbruce
Copy link
Member Author

@UziTech: "Let the editor figure it out" - agreed...that's why I fell in love with .editorconfig - it let's the project tell the editor what to do. 😊

I have a project that is a blend or PHP, JS, Sass, and so on. PHP uses spaces not tabs - four spaces for each (if you are following the PHP-fig coding standards, which is common for the community). The JS and CSS communities seem more inclined to 2. As long as contributors use an editor that understands them, everyone is doing the same thing without having to wait for the linter to tell them they're wrong. 😆

So, yeah, .editorconfig for the win.

.editorconfig Outdated
indent_style = space
indent_size = 2

[*.md]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't impose rules on markdown files because we use them to make tests, and sometimes we want to break out of all these rules. Is there a way to apply this only to documentation files?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. This could cause problems if it is not limited the docs.
Maybe remove this check for *.md and it can be added when we create a /docs folder with GitHub Pages.

@Feder1co5oave
Copy link
Contributor

Personally I use tabs for indentation with 4 spaces width (or 2 spaces width for javascript most of the time) but it doesn't really matter to me either.
I use Sublimetext3 and I configured it by clicking on "tab size: ?" in the bottom right corner. There you click on "use spaces for indentation" and "tab width: 2". If you screwed something up before setting this, you can click on "convert indentation to spaces" and it'll magically fix everything for you.

The beauty is that if you use tabs in marked.js or test/index.js, eslint complains:

$ npm run lint

> marked@0.3.12 lint /home/federico/marked
> eslint --fix lib/marked.js test/index.js


/home/federico/marked/test/index.js
  119:2  error  Mixed spaces and tabs                 no-mixed-spaces-and-tabs
  119:4  error  Unexpected tab character              no-tabs

.editorconfig Outdated
indent_style = space
indent_size = 2

[*.md]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. This could cause problems if it is not limited the docs.
Maybe remove this check for *.md and it can be added when we create a /docs folder with GitHub Pages.

@Feder1co5oave
Copy link
Contributor

Feder1co5oave commented Feb 26, 2018

Note: you need a plugin to have this working in SublimeText, Atom, Emacs, gedit, Eclipse, Notepad++, Vim, VSCode and others.

@Feder1co5oave
Copy link
Contributor

Also, should we include a brief "coding style" section in our "contributing" documentation (be it in README on in some other file)?

@joshbruce joshbruce mentioned this pull request Feb 26, 2018
7 tasks
@joshbruce
Copy link
Member Author

Agreed on the markdown believe we can do exclude...will resubmit shortly. Also agreed on the needing a plug-in in most cases (vendors not supporting it natively); however, we don't even have the option to support it right now. :)

I was first introduced to it only a couple of years ago and now I don't leave home without it. 😉

https://github.com/editorconfig/editorconfig/wiki/Projects-Using-EditorConfig - It's gaining traction in a lot of projects I trust - especially the A11Y.

@joshbruce joshbruce mentioned this pull request Feb 27, 2018
@joshbruce
Copy link
Member Author

Also, found out the VIM can't support it period while working #1083 - so, I've updated #1081 with a checkpoint to run npm test prior to submitting the PR, which will run npm test:lint now and instructions related to our NPM scripts, namely running npm run lint to have the linter fix any lint issues.

See also #1089 - just created it.

@styfle styfle merged commit 3f04f92 into markedjs:master Mar 2, 2018
zhenalexfan pushed a commit to zhenalexfan/MarkdownHan that referenced this pull request Nov 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RR - refactor & re-engineer Results in an improvement to developers using Marked, or end-users, or both.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants