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

fix(.editorconfig): make C files save w/o BOM #90

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

vwheeler63
Copy link
Contributor

Since my development workstation is on a Windows platform, when I make modifications/fixes to LVGL library code, I often like to do it in this project first because it offers a great place to test the results.

I found that when I did that and then copied the changes to the LVGL project fork I have, they were coming with BOMs in the files, and the LVGL repository maintainers are trying to keep the BOMs out of the source files because they cause problems in some places. File format preferred is [UTF-8] instead of [UTF-8 with BOM].

I offer this as help, since most of the C source files under these projects is from the LVGL repository.

Resolves #89

Since my development workstation is on a Windows platform,
when I make modifications/fixes to LVGL library code, I often
like to do it in this project first because it offers a great place
to test the results.  I found that when I did that and then copied
the changes to the LVGL project fork I have, they were coming
with BOMs in the files, and the LVGL repository maintainers are
trying to keep the BOMs out of the source files because they cause
problems in some places.  I offer this as help, since most of the
C source files under these projects is from the LVGL repository.
@MouriNaruto
Copy link
Collaborator

In my opinion, we should only make LVGL library source files use UTF-8 without BOM.

Using UTF-8 with BOM to save text files will improve the user experience in Windows, especially for non-English platforms.

Also, we also need to modify the project generator to make LVGL library source files compiled with forcing UTF-8 encoding option enabled to ensure MSVC uses UTF-8 instead of other non-English encodings.

So, I think I will do these changes by myself in the recent days.

Kenji Mouri

@vwheeler63
Copy link
Contributor Author

So, I think I will do these changes by myself in the recent days.

Hi, @MouriNaruto !

Are you thinking of adding an .editorconfig at the top level of the lvgl/lvgl repository so it prevents the UTF-8 + BOM in that subdirectory?

--Vic

@MouriNaruto
Copy link
Collaborator

So, I think I will do these changes by myself in the recent days.

Hi, @MouriNaruto !

Are you thinking of adding an .editorconfig at the top level of the lvgl/lvgl repository so it prevents the UTF-8 + BOM in that subdirectory?

--Vic

No, I mean the .editorconfig at the top level of this repository's directory.

Kenji Mouri

@vwheeler63
Copy link
Contributor Author

No, I mean the .editorconfig at the top level of this repository's directory.

I see. The one I edited.... Please enlighten me: how were you thinking of switching only the LVGL source files to UTF-8? (I thought I accomplished that, but if you know a more appropriate way, I'm interested in learning in the interest of improving my competence with .editorconfig files.)

@vwheeler63
Copy link
Contributor Author

Hi, @MouriNaruto . In case you didn't see the above, I was hoping, if you would be so kind, that you would show me what you were thinking.

@MouriNaruto
Copy link
Collaborator

MouriNaruto commented Nov 25, 2024

No, I mean the .editorconfig at the top level of this repository's directory.

I see. The one I edited.... Please enlighten me: how were you thinking of switching only the LVGL source files to UTF-8? (I thought I accomplished that, but if you know a more appropriate way, I'm interested in learning in the interest of improving my competence with .editorconfig files.)

In your current implementation, you changed the editorconfig rule to make all C/C++ source and header files use UTF-8 without BOM in this repository. I hope we only limit the UTF-8 without BOM in LVGL source and header files. (Other files will still use UTF-8 with BOM for better compatibility under Windows with non English environments.)

Kenji Mouri

@vwheeler63
Copy link
Contributor Author

you changed the editorconfig rule to make all C/C++ source and header files use UTF-8 without BOM in this repository.

cc: @kisvegabor

Ah! Thank you for the clarification. Since there are no other applicable .editorconfig files, the only options to do that are to:

  • add an additional .editorconfig file under the LvglPlatform directory, or
  • add one to the LVGL repository itself.

So I reverted the change to the root .editorconfig file and added a smaller one under the LvglPlatform directory that leads to the LVGL sub-module and will thus keep its files UTF-8 without BOM. Unfortunately, it will also apply to the FREETYPE sub-module, but perhaps that is a good thing?

The only (clean) alternative is to add one to the LVGL repository itself. (I say "clean" because if it is added there only under this repository, it will show up in the LVGL sub-module as an "Untracked file" when the sub-module is opened [or if you are using git from the command line and cd into the LVGL sub-module].)

@vwheeler63
Copy link
Contributor Author

Hi, @MouriNaruto !

In case you hadn't seen it, is this PR now in alignment with what you were thinking?

Kind regards,
Vic

@kisvegabor
Copy link
Member

I'm a little worried if the editorconfig and astyle will conflict. It might be cumbersome to make the produce the same result in all environments.

@vwheeler63
Copy link
Contributor Author

vwheeler63 commented Nov 28, 2024

I'm a little worried if the editorconfig and astyle will conflict. It might be cumbersome to make the produce the same result in all environments.

As far as I know, that .editorconfig will only suppress the BOM at the beginning of the file and will have no other effect. I have never heard of it having any impact on style except for how the TAB key is interpreted (in this case it is leading spaces).

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.

BOMs Saved into C Source Files
3 participants