Hi Friedrich,
Very nice, thanks for doing this!
These are the few nit-picks (feel free to ignore them) I have:
markdownview.cpp:55
The `hasSelection` variable makes the code look much more complex than it is.
It can be removed and the last argument of the `Q_EMIT contextMenuRequested`
call set to:
!linkUrl.isValid() && hasSelection()
markdownpartfactory.cpp:45
Personal preference - use `auto` when you have `= new Something(:::)` on the
right - no need for `Something` to be written twice:
Something* p = new Something(...)
The variable part can even go away - just return new MarkdownPart.
Similar lines exist in markdownpart.cpp, though there you use auto almost in
all these cases.
markdownbrowserextension.cpp:51
There is no point in having the `const bool forcesNewWindow = false` declared
at the beginning of the function when it is used only in a single line at the
end of the function:
bargs.setForcesNewWindow(forcesNewWindow);
Replacing this with false is IMO more readable as you don't need to scroll up
to check what the value of forcesNewWindow is:
bargs.setForcesNewWindow(false);
Cheers,
Ivan
--
dr Ivan Čukić
i...@cukic.co, https://cukic.co/
gpg key fingerprint: 8FE4 D32F 7061 EA9C 8232 07AE 01C6 CE2B FF04 1C12
