OtterBrowser / otter-browser

Otter Browser aims to recreate the best aspects of the classic Opera (12.x) UI using Qt5
https://otter-browser.org
GNU General Public License v3.0
1.8k stars 277 forks source link

general code style issues, needs rework #1650

Closed hckme4 closed 4 years ago

hckme4 commented 4 years ago

I'm noticing a lack of code style, letting lines go past 150 characters and generally failing to clean up the code. It needs some rework to allow for easier debugging of issues.

hckme4 commented 4 years ago

Please assign this to myself, attached urls are pastes of diffs I'm working on cleaning up. Currently cleaned up the general style of src/main.cpp, src/core/ActionExecutor.cpp, src/core/ActionExecutor.h, and src/core/ActionsManager.cpp.

Working on adding comments to this code and and reducing unnecessary things such as ternary operators for better taste.

src/main.cpp diff: https://pastebin.com/DCWVj9g0 src/core/ActionExecutor.cpp diff: https://pastebin.com/W8wV5qZb src/core/ActionExecutor.h diff: https://pastebin.com/W6ZvhS1J src/core/ActionsManager.cpp diff: https://pastebin.com/EUU5xdEc

NOTE: THIS IS A WORK IN PROGRESS. THIS IS BY NO MEANS FINISHED

Frenzie commented 4 years ago

Having a slightly different personal style preference is of course not quite the same thing as there being a lack of style. ;-)

It might be helpful to try to be more specific about what kind of issues you were facing while debugging.

https://github.com/OtterBrowser/otter-browser/blob/f20a12a85893d4ef5b976004b63828e8b3678c95/CONTRIBUTING.md

Avoid long lines but also try not to break up if-statements etc. Any good editor should be able to wrap long lines.

A sample of a change in the diff above:

-void otterMessageHander(QtMsgType type, const QMessageLogContext &context, const QString &message)
-{
+void otterMessageHander(QtMsgType type,
+        const QMessageLogContext & context,
+                const QString & message) {

Incidentally, I'm reminded of http://lkml.iu.edu/hypermail/linux/kernel/2005.3/08168.html

Frenzie commented 4 years ago

@hckme4 You could try popping by on IRC to see if @Emdek's around.

Emdek commented 4 years ago

@hckme4, we are using Allman style with tabs (as mentioned in file linked by @Frenzie): https://en.wikipedia.org/wiki/Indentation_style#Allman_style I find it being the most readable one.

About line length, even Linus recently removed 80 characters limit for the kernel (AFAIK it's 100 now), we should strive to find better ways to reduce their length, instead of doing nasty line breaks.

Moreover, any mass (mostly) white space change is pointless and only makes it more difficult to navigate history.

Emdek commented 4 years ago

As noted by @Frenzie, it's a good idea to ask first on IRC. ;-)