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

Docs: refine v2-faq.md and fix a few typos #1617

Closed
wants to merge 4 commits into from
Closed

Docs: refine v2-faq.md and fix a few typos #1617

wants to merge 4 commits into from

Conversation

MrChocolatine
Copy link

@MrChocolatine MrChocolatine commented Oct 1, 2023

As I was reading the changes in #1607 and #1613 (thanks @simonihmig), I took the opportunity to refine a few things on this document.

Initially I spotted a few typos, then decided to improve the overall readability of the document.

@MrChocolatine MrChocolatine changed the title Docs: refine v2-faq.md Docs: refine v2-faq.md and fix a few typos Oct 1, 2023
Copy link
Collaborator

@simonihmig simonihmig left a comment

Choose a reason for hiding this comment

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

Thanks for working on this!

I just wonder about the manual line breaks added here. Is that something we really still have to do nowadays? Like editors can wrap lines. And if you have explicit line breaks, editing the text can become very tedious, right?

We have prettier explicitly disabled for .md files in this repo. So I think either we set up prettier to do this stuff for us, or we don't care!? Thoughts?

@NullVoxPopuli
Copy link
Collaborator

And if you have explicit line breaks, editing the text can become very tedious, right?
We have prettier explicitly disabled for .md files in this repo. So I think either we set up prettier to do this stuff for us, or we don't care!? Thoughts?

ye, if we merge this I'd like to have this style of formatting be automatic.
it is tedious to do manually.

Copy link
Collaborator

@NullVoxPopuli NullVoxPopuli left a comment

Choose a reason for hiding this comment

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

can prettier do this for us?


```ts
import { Something } from './path/to/file.ts';
```

If you've done ESM in node, this should feel familiar, and we can be be consistent with JS imports as well:
If you've done ESM in node, this should feel familiar, and we can be be consistent with JS imports
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's fix be be.

Copy link
Collaborator

@lolmaus lolmaus left a comment

Choose a reason for hiding this comment

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

Me, @mansona and @void-mAlex agree that line breaks add little to no value and cause problems:

  • bloated diffs (both for current and upcoming PRs),
  • difficulty reading the source for dyslexic people,
  • difficulty editing and reformatting.

We believe a paragraph should be a paragraph. You can tweak your IDE for a line wrap (for a specific extension) and preferred line width.

@MrChocolatine
Copy link
Author

line breaks add little to no value and cause problems:

  • bloated diffs (both for current and upcoming PRs),
  • difficulty reading the source for dyslexic people,
  • difficulty editing and reformatting.

It actually facilitates reading.

@MrChocolatine MrChocolatine deleted the docs-update-v2-faq branch October 11, 2023 14:41
@mansona
Copy link
Member

mansona commented Oct 11, 2023

Hey @MrChocolatine 👋 I just want to explain the comment that @lolmaus made a little bit more. We are currently in a meeting for the Embroider Triage and Andrey is "driving" so he expressed an opinion that I had with his comment but it is not exactly clear the reasons for that opinion so I would like to explain a bit better.

While you are correct that sentence length is important for facilitating reading, there is a difference between the display of line length and forcing line length by manually (or automatically) putting newlines in the source material. I'm speaking as someone with dyslexia that this sort of forced line break will make it significantly harder for me to read the Markdown source, so much so that I would be forced to read only a rendered output and not be able to easily read diffs in things like github.

I personally appreciate the changes that you were making in this PR so I would like to try to get it merged if you wouldn't mind re-opening it and removing the linebreak change?

@lolmaus
Copy link
Collaborator

lolmaus commented Oct 11, 2023

I would like to point out that:

  • The length of a sentence and the length of a line are separate and unrelated things.
  • Different users may have different preferences for optimal line width.

I believe, each developer can and should tweak their IDE for their preference without imposing it on everyone else, as different people may have different needs. For example, if the width of my IDE pane fits 80 characters, text with forced line breaks will look horrible.

Here's how your PR's diff looks like on my portrait screen:
image
As you can see that the original text is nicely formatted and the proposed is messed up.

It is trivial to achieve the desired line width in an IDE by enabling word wrap and setting a certain width to your editor pane. So you can enjoy your desired line width without interfering with other people.

That said, we are positively interested in your other changes in this PR. Can you please amend the commit to contain typo fixes and other changes without the extra line breaks?

And thank you for you effort! 🙏

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

Successfully merging this pull request may close these issues.

None yet

5 participants