search

ffmpeginteropx / FFmpegInteropX

FFmpeg decoding library for Windows 10 UWP and WinUI 3 Apps
Apache License 2.0
205 stars 52 forks source link

editorconfig: Remove unnecessary rules #433

Closed softworkz closed 1 week ago

softworkz commented 2 weeks ago

None of these rules are needed for Visual Studio and these also do not stem from any defaults/templates of VS.

softworkz commented 2 weeks ago

This might also solve some of the problems which @JunielKatarn is having.

It should further avoid unrelated whitespace changes and end-of-file changes in future PRs.

softworkz commented 2 weeks ago

Right. The crlf there, in combination with not using autocrlf setting is probably causing part of the problems @JunielKatarn has.

He's working on Linux and using a different IDE (Linux because he said it and different IDE because there's no VS for Linux). I'm very thankful that you accepted some changes that helped me for my use cases and I'm all into helping him to get his scenario working as well. I never meant to be rejective, it's just that I do have that shitload of experience that I know upfront from many commits, which problems these will (or may) cause in the future. It would be easier if he would be a bit more open about his setup, but based on all that is known by now, this should significantly improve his use case and I hope this PR also makes it clear that I want to help rather than block or reject. 😄

brabebhin commented 2 weeks ago

If i had to guess he is using Rider.

softworkz commented 2 weeks ago

Since he appears to be affiliated with MS in some form, a very steep speculation would be a secret prototype of a "VS for Linux". It's the most requested topic of all times on the MS Dev Community site (officially rejected though). That would explain why he doesn't tell (not allowed). ;-)

brabebhin commented 2 weeks ago

But Microsoft has its own version of this library. I don't think he'd bother with us in a work context.

softworkz commented 2 weeks ago

But Microsoft has its own version of this library. I don't think he'd bother with us in a work context.

I don't mean he works on it, it would be about dog-feeding (letting employees from different projects use it for testing).

JunielKatarn commented 2 weeks ago

Hi!

I'm currently out of town and away from keyboard, so haven't had time to review feedback on my PR nor look into this discussion, but notifications show I've been alluded a few times so I'll post some clarifications in my next reply...

softworkz commented 2 weeks ago

In case there's something you wouldn't like to see written here, just let me known and I'll delete it.

JunielKatarn commented 2 weeks ago

Let me start saying I do have issues with this PR.

Unless we don't want to use an editor helper tool (i.e. EditorConfig) at all and let whatever formatting issues and potential conflicts happen, we shouldn't drop the whitespace trimming rule. I won't press hard on this, but don't understand the value of removing the automated editor formatting.

Now, into the mentions:

Right. The crlf there, in combination with not using autocrlf setting is probably causing part of the problems @JunielKatarn has.

Partially. It all comes down to that line endings we want for what files. Issues will happen even if I set AUTOCRLF on Windows (which I have for this project). AUTOCRLF forces CRLF even for files that are meant to be LF. Won't get into this topic here. I will open a discussion next week.

Right. The crlf there, in combination with not using autocrlf setting is probably causing part of the problems @JunielKatarn has.

Not exactly. It's only disabling AUTCRLF that induced the huge diffs. This was intentional (I intended to make VS-specific project files CRLF in the repository as opposed to LF). Again, won't go further into it here.

He's working on Linux and using a different IDE (Linux because he said it and different IDE because there's no VS for Linux).

macOS to be specific, but the same semantics apply to Linux. Not a different IDE, I actually use VS Code for everything that does not require compiling and running the project (then I open Visual Studio on Windows). I have synchronized settings so the behavior is identical in all OSes.

It would be easier if he would be a bit more open about his setup

Happy to share any further details. If it helps, generally speaking:

JunielKatarn commented 2 weeks ago

Since he appears to be affiliated with MS in some form

I am indeed affiliated with MS in some form :) However, my involvement in this project is 100% out of personal interest and using only personal resources.

a very steep speculation would be a secret prototype of a "VS for Linux".

I wish! Sadly, not only not the case, but there was a Visual Studio for Mac that is getting retired literally today.

It's the most requested topic of all times on the MS Dev Community site (officially rejected though). That would explain why he doesn't tell (not allowed). ;-)

Sorry if I missed any direct questions on this regard. I was very sporadically available this week. Based on my previous answer, I would guess there are just no plans to develop other ports for Visual Studio. My guess is VS Code is the intended to be a cross-platform IDE (it does support .NET, CMake and actual C++ on all platforms).

softworkz commented 2 weeks ago

Unless we don't want to use an editor helper tool (i.e. EditorConfig) at all and let whatever formatting issues and potential conflicts happen, we shouldn't drop the whitespace trimming rule. I won't press hard on this, but don't understand the value of removing the automated editor formatting.

The reason for removing it is specifically about your submissions which are containing those whitespace changes which don't belong into commits/PRs with functional changes because the whitespace changes are distracting from the actual changes (like I and @lukasf said in response to your PR). Same goes for the line break at end of file rule.

Now, I don't know VSCode well enough regarding .editorconfig behavior, but on 2 or 3 occasions, you mentioned that your editor would apply editorconfig rules to the whole file when saving a file. And that's a real problem, because it means that you aren't easily able to work in a way that you can commit functional changes which do not include editorconfig-based changes to lines which don't carry any functional change.

Visual Studio is very different here: It applies editorconfig rules only to the lines which you are actually editing and doesn't apply those rules to any other line. And it applies those rules "live" while editing, not on save. If everybody would be using Visual Studio only, then we could easily keep those rules. But when it causes you to submit PRs which include editorconfig changes to all lines in a file, then that's a problem and it's better to not have those rules at all than having to discuss this it each time.

Now for the line endings: I think we are all good about the autocrlf setting at the Git side. You have it false on Linux and true on Windows (at least for this repo) - so that's already perfectly in line with what we all are doing.

The second side regarding line endings is the end_of_line setting in editorconfig. You had added the editorconfig file in a PR like 2 years ago, setting it to LF for all files. Two weeks later, @lukasf had changed it to CRLF presumably because he experienced those kind of problems that I've been talking about all the time (if you haven't done yet, I'd kindly ask you to watch my 2 part video response on this subject). The setting of end_of_line=CRLF (applied to all files *.*) doesn't hurt on Windows, but isn't it that what's troubling you? Your argument to set end_of_line=LF for editorconfig files was that the default for editorconfig would be CRLF, but that's not a default per spec, that's just the line above (for *.*). This PR removes that line and all other end_of_line rules, so that should solve a large part of your problems. If there are specific files which you need to be CRLF (even on MacOS), then we can add such specific rules to editorconfig, because setting end_of_line to CRLF doesn't hurt in Windows, but setting it to LF does (see my videos).

softworkz commented 2 weeks ago

Since he appears to be affiliated with MS in some form

I am indeed affiliated with MS in some form :) However, my involvement in this project is 100% out of personal interest and using only personal resources.

macOS to be specific, but the same semantics apply to Linux. Not a different IDE, I actually use VS Code for everything that does not require compiling

Thanks a lot for sharing all this info. This allows to get a better understanding of each others workflow and setup and makes it so much easier to find a good solution which works well for everybody! I'm sure we'll be able to figure something out.

softworkz commented 2 weeks ago

@JunielKatarn - from my research, it appears that VSCode itself does not apply those rules like whitespace trim to all lines when saving. It only applied this to edited lines (just like the real Visual Studio). Can you confirm this?

I further read that there's a "Editorconfig for VS Code" extension which causes VS Code to apply rules to all lines when saving - do you have that extension installed?

lukasf commented 1 week ago

Oh this one got automatically closed as I merged+deleted the winui branch. Please Create a new PR against the master branch.

softworkz commented 1 week ago

Oh this one got automatically closed as I merged+deleted the winui branch. Please Create a new PR against the master branch.

Done: https://github.com/ffmpeginteropx/FFmpegInteropX/pull/436