Conversation
|
After quickly scrolling through the diff I think I'm going to be ok with merging this PR, but I'm not on board with getting rid of these lines everywhere.
They might indeed be a bit inconsistent at the top or bottom of a file (or class declaration), but there should always be such a line between function definitions or different namespaces. In header files these lines are not needed when the class or function below has documentation.
This is going to be very personal, because I actually really like having those lines when scrolling through code. I don't need the lines, but they do help me to more quickly end at the correct spot when scrolling back and forth in a file. Right now there is an empty line above and below the commented line. I am willing to accept removing the empty line above and only keeping the commented line and one empty line below inbetween functions.
I don't really want to go below 135. The absolute lowest value I would accept right now is 130. While I am willing to reduce the length of the "//////" lines a bit (to e.g. to 100 characters instead of 130), these lines also indicate the length available for writing documentation in header files. And I like to keep it that way, so in practice I don't really want to make them shorter than 120 characters there. |
This is a followup to commit e48b7c4. It removes completely pointless comment lines at ends of classes, namespaces, functions and files.
|
Rebased on top of current 1.x branch. |
Fair enough. Let's just merge this commit then for now.
If you are OK with fixing the inconsistencies, then I'll keep an eye out for those and fix them up as I go (adding or deleting as needed) in future PRs. I'm very aware that having the comments or not and the horizontal length of them is very much a personal preference issue. I just brought it up to have the discussion. I do see your point that they mark the max width of comments and the like - fine. Personally I think they are too wide (but that goes for all of the code) since even though I'm using a 4K monitor, I still see lines wrap in my editor when I use a split screen setup - that is probably (at least partly) due to me using a fairly large font size since my eyesight is not the greatest, but it's also due to me believing that allowing long lines encourage deep nesting which I'm not a fan of. But, I see your point and I am aware that we are discussing something very minor, so let's just leave it at 135 as you prefer and move on to more important stuff :-)
Let's just leave that as-is. What annoyed me the most was actually the lines between functions, but since you like those let's just not change anything. I'll be pleased if you merge this since that at least gets rid of some completely pointless comment lines at the end of functions, classes, scopes etc and get rid of some needlessly duplicated lines. That's a nice little cleanup in my book. Going forward I may add or remove a few comment lines to make things more consistent, but let's not waste more time on whether or not they should be there at all and their length - your project, your rules - I stated my case, you have sane objections, let's leave it at that :-) Kind regards, |
Hi Texus
As the commit message says, This is a followup to commit e48b7c4. It removes completely pointless comment lines at ends of classes, namespaces, functions and files and where sometimes duplicated.
This removes more than 500 completely pointless comment lines and I propose this as a starting point for getting rid of even more (hopefully all).
These "//////////" comment lines between functions are, in my opinion, completely pointless - let me explain:
They are not used consistently (at all), so when changing or adding code it is unclear whether they should be added or left out.
They increase the vertical length of the code for no good reason. I doubt any competent developer needs a "//////////" line to see where one function ends and another begins - a blank line between functions is sufficient, so the comments are just more stuff to scroll through and less relevant code on screen.
They increase the file size for no gain at all. Sure, a few extra bytes that are not language tokens don't take much time to parse, but thousands of lines and even more characters do add up.
When reading diff's the "//////////" lines in the context parts of the diff is quite annoying - it makes what you have to read longer, you have to scroll, it adds nothing of value, the diff file is larger, etc.
If we get rid of all the pointless comment lines, then we could maybe reduce the clang-format line break below 135 characters which would be nice for split screen views in many IDEs/editors.
I am aware that this is very much a personal preference thing and subjective, so I want your input and just post this commit as a discussion point.
If you hate it - reject it.
If are OK with it as-is, feel free to merge it.
If you love it and want more; merge it and let me know ;)
Looking for feedback..
Kind regards
Jesper Juhl