lvgl / lv_port_pc_visual_studio

Visual Studio projects for LVGL embedded graphics library. Recommended on Windows. Linux support with Wayland is work in progress.
MIT License
607 stars 306 forks source link

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

Closed vwheeler63 closed 2 days ago

vwheeler63 commented 1 week ago

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

MouriNaruto commented 1 week ago

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 commented 1 week ago

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 commented 1 week ago

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 commented 1 week ago

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 commented 6 days ago

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 commented 6 days ago

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 commented 6 days ago

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:

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 commented 4 days ago

Hi, @MouriNaruto !

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

Kind regards, Vic

kisvegabor commented 4 days ago

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 commented 3 days ago

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).

kisvegabor commented 3 days ago

IT could be okay in this case. Please send a PR to LVGL.

vwheeler63 commented 3 days ago

IT could be okay in this case. Please send a PR to LVGL.

Are you thinking perhaps to have one in the root directory?

MouriNaruto commented 2 days ago

@vwheeler63 I think we should change the root .editorconfig to this repository, to add the new item for that to ensure the IDE can load the policy.

Kenji Mouri

MouriNaruto commented 2 days ago

I have made a commit https://github.com/lvgl/lv_port_pc_visual_studio/commit/27cbbb7020b5f9db705d014af9acb74f171d5c0c to fix the issue which this PR fixes.

Kenji Mouri

MouriNaruto commented 2 days ago

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.

Done. See https://github.com/lvgl/lv_port_pc_visual_studio/commit/67c3c1f21a65c8a540b67f84440cf783b377ce6c for more information.

Kenji Mouri

vwheeler63 commented 2 days ago

I have made a commit 27cbbb7 to fix the issue which this PR fixes.

Done. See 67c3c1f for more information.

Kenji Mouri

Excellent! Thank you, @MouriNaruto !