search

plone / volto

React-based frontend for the Plone Content Management System
https://demo.plone.org/
MIT License
427 stars 575 forks source link

Editorconfig max_line_length should be removed or set to 80 #4772

Closed pnicolli closed 11 months ago

pnicolli commented 11 months ago

Describe the bug See https://github.com/plone/volto/pull/3503 This PR introduced new .editorconfig files, which is very good, but sets max_line_length = off, which is read by prettier and every file we save after that change is then completely rewrote. We also get errors from eslint(prettier/prettier) since it still believes lines should be 80 chars long.

To Reproduce Steps to reproduce the behavior:

  1. Open a js file with a lot of code, for example src/config/ControlPanels.js
  2. Save without making changes (you need an editor properly set with eslint and prettier)

I would suggest removing that line from .editorconfig files, but also setting a default is fine. The default should be 80 imho, otherwise we need a full run of linters fix on the whole repo and I would suggest against that.

Himanshu-370 commented 11 months ago

image

@pnicolli Here, the max_line_length setting is added specifically for JavaScript and JSX files ([*.{js,jsx}]). The value is set to 80, as you indicated in your message. This ensures that Prettier and ESLint respect the line length limit of 80 characters for these file types.

stevepiercy commented 11 months ago

@Himanshu-370 please read and follow Contributing to Plone and First-time contributors.

pnicolli commented 11 months ago

This was actually already fixed last week. I didn't link the closing PR properly so this remained open. My bad, sorry. This was the PR: https://github.com/plone/volto/pull/4776

Himanshu-370 commented 11 months ago

@pnicolli Happy to know it was fixed. I just wanna know was my solution correct? I am a beginner.

pnicolli commented 3 weeks ago

@Himanshu-370 sorry your reply ended up somewhere among a ton of other email and I overlooked it. You solution was correct but not perfect imho. If you see the linked PR above #4776 the best solution imho is to remove the config so the editor will use the max line length defined with other prettier config rules.