Clarification on Formatting Policy in the Repository
@saicaca
Hi, I’d like to clarify the current formatting policy in this repository.
The recent merges of #368 and #386 (or similar) have introduced many changes that appear to stem from individual VSCode settings (e.g., tabs vs. spaces), which has caused some inconsistencies in the formatting.
When "diffEditor.ignoreTrimWhitespace" is set to true, VSCode may not show formatting changes such as tabs or trailing spaces. However, Git still recognizes these changes, which can result in large diffs that may be overlooked locally but appear in pull requests. You can see this issue visualized in #401 for reference.
Were these changes made with approval from the repository owner?
To avoid further discrepancies, I think it’s important to clearly define:
- Mixing tabs and spaces is allowed, or
- Mixing tabs and spaces is not allowed.
Either is fine by me, but the current ambiguity is leading to unnecessary diffs and confusion.
If (1) is intended, I suggest either:
- Including the relevant settings in the repository, or
- Clearly documenting the formatting expectations
This is not a matter of personal preference, as suggested by that collaborator, but a request for consistency in an open-source project.
close #389
Additional notes:
It is essential to ensure consistency within the project. In addition to the files used as examples in #401, a comprehensive review and reformatting of all relevant files in the project is recommended.
Committers should only be responsible for the changes they make in their PRs, and not for other modifications outside of their changes.
Now, it's not fair to be treated as the bad guy all together, so let's first correct the mistake.
1
The recent merges of #368 and #386 (or similar) and the introduction of many changes based on a collaborator’s personal VSCode settings (e.g., tabs vs. spaces) have created some inconsistencies.
The "inconsistency" has been occurring for a while. As for Astro files, only those in the code fence (script processing related) are processed.
This is described in the biome documentation. Therefore, some Astro files are formatted and some are not. At this point, the "inconsistency" you mentioned has occurred.
https://biomejs.dev/internals/language-support/
2
This is not a matter of personal preference, as suggested by that collaborator, but a request for consistency in an open-source project.
Yes, that is correct, but please note that I was the one who introduced biome in order to provide some degree of "consistency."
And there was an error in that "consistency," which has now simply been corrected.
In the early days of this project, there wasn't even a formatter or a linter.
I can't say for sure that this isn't a change that was made for my own "preference" (in fact the initial biome settings were personal), but it's just a change to maintain the "consistency" you mentioned.
others
So, what I mean is, I don't know what level of "consistency" you mean, but at least it meets the consistency required for this project. (Actually, I remember that saicaca was working without ESLint or Prettier.)
A few more
I've mentioned it in the discussion, but he's busy and won't be back anytime soon,
so I'm signing on behalf of the repository owner.
https://github.com/saicaca/fuwari/discussions/356
In case anyone is wondering about the statement "changes of a few to a dozen lines," I'd like to say that "small changes" does not necessarily mean just the number of lines. It also includes changes that have small "cognitive load" and "purpose of the change."
Well, since I got a mention on saicaca, I'll leave a comment just in case.
If you feel there is something wrong with this change, feel free to remove yourself as a collaborator and continue contributing.
@L4Ph
I'm focusing on the current situation and the direction moving forward, rather than the past.
I created a separate issue to get confirmation from the repository owner. Why are you adding additional commentary here?
I’d prefer to keep the issue concise. Could we discuss this elsewhere?
@saicaca
Issue #389 became quite long, so I'll just summarize the main points here.
- I noticed that saving a file in a freshly cloned environment caused a large number of diffs.
- The main change was that spaces were converted into tabs, so I focused my investigation on this.
- To identify the root cause, I opened
#389. #389 - The collaborator suggested that
formatOnSavein.vscode/settings.jsonmight be responsible. - If that were the case, the issue should affect more than just
astro.config.mjs, so I reviewed all files and compiled a list of affected ones. - From the commit history, I confirmed that the space-to-tab conversion came from
#368, though this itself wasn’t the core issue. #368 - I followed several suggestions from the collaborator, but the problem remained unresolved.
- Later, the collaborator shared a screenshot of the diff view, which was different from mine.
- When I asked about this discrepancy, the collaborator explained that it was due to their personal VSCode settings.
- I asked if those settings were included in
fuwari, and they confirmed they were not. - This revealed that the inconsistency came from unshared editor settings, so I created this new issue to clarify the situation.
It seems that the issue is code formatting causing code conflicts. This project didn't strictly enforce code formatting before since I'm not very skilled in frontend engineering. I think it's ok to establish a standard now, which should have been done earlier. Sorry for the inconvenience. And I agree that we should be more careful when making such global changes in the future to avoid conflicts with existing PRs.
You don’t need to update your branches for now, as I don’t have time to review the new features at the moment. I'll reply to you when I'm ready. If it's inconvenient for you at that time, feel free to let me deal with the conflicts.