Kdesktop 968 applied c++20 style and new features #174

herve-er · 2024-06-17T11:58:11Z

This PR aims to do a first pass of refactoring to applied c++20 recommended style to our code base.

This reverts commit 6f6b73b.

This reverts commit e642f56.

…to enum class

…ementation.

luc-guyot-infomaniak · 2024-06-18T06:21:07Z

src/gui/proxyserverdialog.cpp

-    {ProxyType::ProxyTypeHTTP, {0, QString(tr("HTTP(S) Proxy"))}}
-    //, { ProxyType::ProxyTypeSocks5, { 0, QString(tr("SOCKS5 Proxy")) } }
+    {ProxyType::HTTP, {0, QString(tr("HTTP(S) Proxy"))}}
+    //, { ProxyType::Socks5, { 0, QString(tr("SOCKS5 Proxy")) } }


Is the commented line still useful?

I don't think. @ChristopheLarchier could you please confirm?

luc-guyot-infomaniak · 2024-06-18T06:22:10Z

src/gui/statusbarwidget.cpp

@@ -239,17 +239,17 @@ void StatusBarWidget::createStatusActionMenu(MenuWidget *&menu, bool &resetButto
            QWidgetAction *syncAction;
            for (auto const &syncInfoMapElt : syncOfCurrentDrive) {
                if (pauseClicked &&
-                    (syncInfoMapElt.second.status() == SyncStatusStoped || syncInfoMapElt.second.status() == SyncStatusPaused)) {
+                    (syncInfoMapElt.second.status() == SyncStatus::Stoped || syncInfoMapElt.second.status() == SyncStatus::Paused)) {


Maybe we could take the opportunity to rename Stoped with Stopped?

Of course, done in commit: dba898f

And commit: 1810e5a

luc-guyot-infomaniak · 2024-06-18T06:25:19Z

src/libcommon/info/proxyconfiginfo.cpp

@@ -21,7 +21,7 @@
 namespace KDC {

 ProxyConfigInfo::ProxyConfigInfo()
-    : _type(ProxyTypeNone), _hostName(QString()), _port(0), _needsAuth(false), _user(QString()), _pwd(QString()) {}


SonarCloud will be grateful to you if you could initialize _type. _port and _needsAuth in the header and remove the default-constructed data members from this initialization list.

Done in commit: c18bc48

src/libcommon/info/syncinfo.cpp

luc-guyot-infomaniak · 2024-06-18T06:31:35Z

src/libcommonserver/network/proxyconfig.cpp

@@ -21,7 +21,7 @@
 namespace KDC {

 ProxyConfig::ProxyConfig()
-    : _type(ProxyTypeNone), _hostName(std::string()), _port(0), _needsAuth(false), _user(std::string()), _token(std::string()) {}


Would you mind applying the usual recommandation for initialization list?

As I used VS2022 to find and replace with regular expressions, I only checked that the applied modifications were correct but didn't check if the modified code needed to be refactored to adapt to our "new" guidelines. Sorry about that and thanks for noticing it 👍🏾.

Done in commit: 3c50feb and 3ee0add

luc-guyot-infomaniak · 2024-06-18T07:58:50Z

src/libsyncengine/jobs/network/uploadjob.cpp

-    if (_linkType == LinkTypeSymlink) {
-        return _targetType == NodeTypeFile ? mimeTypeSymlink : mimeTypeSymlinkFolder;
-    } else if (_linkType == LinkTypeHardlink) {
+    if (_linkType == LinkType::Symlink) {


Nice to have: refactor with a switch.

Done here: a56cf19

luc-guyot-infomaniak · 2024-06-18T09:41:11Z

src/libsyncengine/update_detection/file_system_observer/remotefilesystemobserverworker.cpp

        valid = CommonUtility::dirNameIsValid(name);
    }

    if (!valid) {
        LOGW_SYNCPAL_DEBUG(_logger, L"The file/directory name contains a character not yet supported by the filesystem "
                                        << SyncName2WStr(name).c_str() << L". Item is ignored.");

-        Error err(_syncPal->syncDbId(), "", nodeId, type, name, ConflictTypeNone, InconsistencyTypeNotYetSupportedChar);
+        Error err(_syncPal->syncDbId(), "", nodeId, type, name, ConflictType::None, InconsistencyTypeNotYetSupportedChar);


What about InconsistencyType::NotYetSupportedChar?

These classes can't be easily converted to enum classes due to the numerous bitwise operations involved. I will take a deeper look to see if I can find an efficient way to convert them to enum classes without making the code unreadable.

You can check these 3 commits 5470bb5, c6f3ed3, 78afc22.

luc-guyot-infomaniak · 2024-06-18T09:42:42Z

src/libsyncengine/update_detection/update_detector/updatetree.cpp

+                   NodeType::Directory, {}, (side == ReplicaSide::Local ? dbNode.nodeIdLocal() : dbNode.nodeIdRemote()),
+                   (side == ReplicaSide::Local ? dbNode.created() : dbNode.created()),
+                   (side == ReplicaSide::Local ? dbNode.lastModifiedLocal() : dbNode.lastModifiedRemote()),
+                   0,  //(side == ReplicaSide::Local ? dbNode.lastModifiedLocal() : dbNode.lastModifiedRemote()),


Is the commented part still relevant?

luc-guyot-infomaniak · 2024-06-18T09:46:25Z

src/server/appserver.cpp

@@ -491,8 +491,8 @@ void AppServer::onRequestReceived(int id, RequestNum num, const QByteArray &para
                sendUserUpdated(userInfo);
            }

-            resultStream << exitCode;
-            if (exitCode == ExitCodeOk) {
+            resultStream << enumClassToInt(exitCode);


Just to check: the result is then translated back to an enum?

It is, here is the client code:

QDataStream resultStream(&results, QIODevice::ReadOnly); resultStream >> exitCode;

where >> operator reinterpret_cast to exitCode:

template <typename T> typename std::enable_if_t<std::is_enum<T>::value, QDataStream &> operator>>(QDataStream &s, T &t) { return s >> reinterpret_cast<typename std::underlying_type<T>::type &>(t); }

luc-guyot-infomaniak · 2024-06-18T09:55:52Z

src/server/vfs/win/vfs_win.cpp

@@ -654,11 +654,11 @@ bool VfsWin::status(const QString &filePath, bool &isPlaceholder, bool &isHydrat

 bool VfsWin::fileStatusChanged(const QString &path, SyncFileStatus status) {
    LOGW_DEBUG(logger(), L"fileStatusChanged: " << Utility::formatSyncPath(QStr2Path(path)).c_str() << L" status = "
-                                                << Utility::s2ws(Utility::syncFileStatus2Str(status)).c_str());
+                                                << Utility::s2ws(Utility::SyncFileInstruction2Str(status)).c_str());


Why using the upper case S in the function name?

My bad, I made the correction in commit: 24387cd

…ool.

…ttps://github.com/Infomaniak/desktop-kDrive into KDESKTOP-968-Applied-c++20-style-and-new-features

…topped)

Co-authored-by: Luc Guyot <[email protected]>

…ttps://github.com/Infomaniak/desktop-kDrive into KDESKTOP-968-Applied-c++20-style-and-new-features

…n a previous commit).

…features

sonarcloud · 2024-06-19T10:02:09Z

Quality Gate passed

Issues
52 New issues
2 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
2.6% Duplication on New Code

See analysis details on SonarCloud

herve-er added 20 commits June 14, 2024 14:52

Replace typedef with using in types.h.

e1b8a98

Revert "Replace typedef with using in types.h."

e642f56

This reverts commit 6f6b73b.

Reapply "Replace typedef with using in types.h."

6a9d0f9

This reverts commit e642f56.

Convert NodeType && ReplicaSide to enum class

c2da0a8

Convert ExitCode & ExitCause to enum class

e9e7a0a

Convert ConflictType& ConflictTypeResolutionto enum class

07e5d1a

Convert CancelType & NodeStatus to enum class

3a6ca4c

Convert SyncStatus & SyncNodeType & SyncDirection to enum class

4b524c7

complete previous commit

230f501

Convert SyncFileInstruction& SyncStep to enum class

cbaba3e

Convert ActionType & ActionTarget & ErrorLevel & Language & LogLevel …

c3e822a

…to enum class

Convert PinState & ProxyType & ExclusionTemplateComplexity to enum class

df7f5b2

Convert PinState IoError to enum class

7f7bc49

Merge from develop.

05fbc13

Replace .find() == .end() with .contains()

dcf8a2a

Replace NULL with nullptr when possible.

ca1d84a

complete previous commit.

fd51bab

Replace .find() != .end() by .contains()

c508f8c

Replace threads with jthreads

9246caa

Replace ERRID macro by std::source_location

0afee12

herve-er marked this pull request as ready for review June 17, 2024 11:58

herve-er requested a review from a team as a code owner June 17, 2024 11:58

github-actions bot assigned herve-er Jun 17, 2024

herve-er marked this pull request as draft June 17, 2024 11:58

herve-er added 6 commits June 17, 2024 14:20

Remove empty line and update utility test to match the new errid impl…

ad3007a

…ementation.

Replace nullptr by 0 where a DWORD is needed.

9eb4391

Bump windows extension from C++17 to C++20.

4ef8912

Fix assert with unwanted implicte conversion to bool of enum.

bf1ab4e

Propagate some non propagated changes of enum to enum class in tests.

89b6f2e

fix implementation of the new Utility::errid function.

15f7688

luc-guyot-infomaniak reviewed Jun 18, 2024

View reviewed changes

herve-er and others added 27 commits June 18, 2024 12:01

Remove unsed effectivePinState

a5c8af5

Add TODO for pinState method on windows.

de99630

Make explicit comparaison instead of implicit conversion of enum to b…

d3eff51

…ool.

Merge branch 'KDESKTOP-968-Applied-c++20-style-and-new-features' of h…

2183da3

…ttps://github.com/Infomaniak/desktop-kDrive into KDESKTOP-968-Applied-c++20-style-and-new-features

Rename SyncStatus::Stoped to SyncStatus::Stopped

dba898f

Complete previous commit. (Rename SyncStatus::Stoped to SyncStatus::S…

1810e5a

…topped)

Initialize member variable in header.

c18bc48

Initialize member variable in header.

3c50feb

Complete previous commit: Initialize member variable in header.

3ee0add

Wrong name for syncFileStatus2Str corrected.

24387cd

Replace accidental change from caps to camel case naming.

9b5fcc6

Small refactor from if to switch.

3f1d1d5

Update src/libsyncengine/jobs/network/abstracttokennetworkjob.cpp

e1c81a6

Co-authored-by: Luc Guyot <[email protected]>

Small refactor from if to switch: upload job.

a56cf19

Merge branch 'KDESKTOP-968-Applied-c++20-style-and-new-features' of h…

c0df07a

…ttps://github.com/Infomaniak/desktop-kDrive into KDESKTOP-968-Applied-c++20-style-and-new-features

Convert InconsistencyType to enumClass

5470bb5

Convert OperationType to enumClass and add operation methods.

c6f3ed3

Reduce verbosity.

78afc22

Remove useless comment. (and move some initialization in header).

1741232

Remove commented code.

309e7fd

Reduce verbosity with "using enum" and add init in if.

95fcc1d

Remove if/else with both action identical.

0fad032

Rename ConflictType2WStr by conflictType2WStr (accidentally renamed i…

9e593c4

…n a previous commit).

Merge branch 'develop' into KDESKTOP-968-Applied-c++20-style-and-new-…

0b7ef19

…features

Merge de develop

812e15a

Fix merge.

cab7b96

Fix merge.

e813073

herve-er marked this pull request as draft June 19, 2024 10:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Kdesktop 968 applied c++20 style and new features #174

Kdesktop 968 applied c++20 style and new features #174

herve-er commented Jun 17, 2024

luc-guyot-infomaniak Jun 18, 2024

herve-er Jun 18, 2024

luc-guyot-infomaniak Jun 18, 2024

herve-er Jun 18, 2024

herve-er Jun 18, 2024

luc-guyot-infomaniak Jun 18, 2024

herve-er Jun 18, 2024

luc-guyot-infomaniak Jun 18, 2024

herve-er Jun 18, 2024

luc-guyot-infomaniak Jun 18, 2024

herve-er Jun 18, 2024

luc-guyot-infomaniak Jun 18, 2024

herve-er Jun 18, 2024

herve-er Jun 18, 2024

luc-guyot-infomaniak Jun 18, 2024

luc-guyot-infomaniak Jun 18, 2024

herve-er Jun 18, 2024 •

edited

Loading

luc-guyot-infomaniak Jun 18, 2024

herve-er Jun 18, 2024

sonarcloud bot commented Jun 19, 2024

Kdesktop 968 applied c++20 style and new features #174

Are you sure you want to change the base?

Kdesktop 968 applied c++20 style and new features #174

Conversation

herve-er commented Jun 17, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

herve-er Jun 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Jun 19, 2024

Quality Gate passed

herve-er Jun 18, 2024 •

edited

Loading