refactor_useQCommandLineParser

[AHeimberger]

Hey Guys, maybe you like this refactoring. It comes with a small issue. In <Windows e.g. 10> the window for showing the help information does not look identically. However I prefer using Qt-Mechanism instead of parsing it free will. Furthermore, if you say it's not direcly necessary to print an information in case the Config-Directory is not valid. It would allow to remove functions like showHint.

Best Regards

[er-vin]

Could you please satisfy the DCO bot? Also I think those three commits can be squashed together.

[AHeimberger]

Could you please satisfy the DCO bot? Also I think those three commits can be squashed together.

I was able to satisfy the "DCO Bot" but I never did a squash on a branch which I already pushed? Can you do a sqashed merge?

[er-vin]

Sure, I can do that.

[AHeimberger]

@er-vin Following lines in application.cpp can be removed as well:

    if (_parser.isSet("version") || _parser.isSet("help"))
        return;	        return;

QCommandLineParser will sys.exit() when asking for version and help, there is no more need for the questioning. I oversaw this, and will do this as well.

[AHeimberger]

Sorry for not getting your PR the attention it deserved last week. Was a bit busy. I like where this is going.

No Problem, I saw there are many refactorings/redesigns ongoing.

[er-vin]

Looking good. I spotted another such hand rolled command line handling in cmd.cpp. Do you fancy porting it as well? If yes I'll wait a bit, if not I'll start reworking a bit the history in there since you said you didn't know how to squash commits.

[AHeimberger]

Looking good. I spotted another such hand rolled command line handling in cmd.cpp. Do you fancy porting it as well? If yes I'll wait a bit, if not I'll start reworking a bit the history in there since you said you didn't know how to squash commits.

Hmm, I have never used the cmd line application, but I was looking at the code and it seems to be feasible. @er-vin There is just one conflict. Option -h and --help is typically used to print help information. The cmd application use -h to Sync hidden files. Any prefrences for this argument? Shall it remain as is?

[er-vin]

Looking good. I spotted another such hand rolled command line handling in cmd.cpp. Do you fancy porting it as well? If yes I'll wait a bit, if not I'll start reworking a bit the history in there since you said you didn't know how to squash commits.

Hmm, I have never used the cmd line application, but I was looking at the code and it seems to be feasible. @er-vin There is just one conflict. Option -h and --help is typically used to print help information. The cmd application use -h to Sync hidden files. Any prefrences for this argument? Shall it remain as is?

Ideally it should remain as is because people using it in scripts should be expected.

[AHeimberger]

@er-vin Hey, do you really think that the option h is used? I am facing following Problem. ./src/cmd/cmd.cpp: bool ignoreHiddenFiles always remains false. Otherwise I do not understand the code. It is initialized as false and when the option (argument) is set it will stay false. I would introduce option i, ignore-hidden instead of h which also does something.

[AHeimberger]

@er-vin I missed to update documentation.

[er-vin]

@er-vin Hey, do you really think that the option h is used?

Well, you can't never know what user do, right? :-)

But it's a tool meant for scripting so I wouldn't underestimate what people do in said scripts.

I am facing following Problem. ./src/cmd/cmd.cpp: bool ignoreHiddenFiles always remains false. Otherwise I do not understand the code. It is initialized as false and when the option (argument) is set it will stay false. I would introduce option i, ignore-hidden instead of h which also does something.

Interesting, so it'd be broken today?

[AHeimberger]

Yes, I broke it, but "I can undo it". I thought I will break it because it has not done anything before and now it is more readable.

[er-vin]

Yes, I broke it, but "I can undo it". I thought I will break it because it has not done anything before and now it is more readable.

Sorry I was being unclear, by "it'd be broken today?" I meant, "it was broken before your changes?"

[AHeimberger]

Yeah, and I was happy to see that you fixed the path concatenation of makeDbName and something else. I was struggling with that while I was testing it. As I fixed it I saw you fixed or merged it a few days ago as well.

[er-vin]

/rebase

[er-vin]

@AHeimberger we're still interested into this, could you please try to rebase and solve the conflicts?

[mgallien]

@AHeimberger we are still interested into your work. Could you have a look at the comment from @er-vin ?

[AHeimberger]

@AHeimberger we are still interested into your work. Could you have a look at the comment from @er-vin ?

I will give it a try. Best Regards

[er-vin]

Do you need help with the rebasing? I still see merge commits in there, that clutters a bit the review.

[AHeimberger]

@er-vin I am not having the time to rewrite the entire history. What I can offer is squashing all commits to one single refactory commit. What do you think? Otherwise I need help.

[er-vin]

Doesn't necessarily need a full rewrite. It needs at least a rebase to get rid of the merge commits.

[AHeimberger]

@er-vin see change, best regards

[AHeimberger]

@er-vin, @mgallien please check. best regards

[mgallien]

@AHeimberger ervin is no longer part of the team unfortunately I was very busy the last two weeks and could not really review your PR will do my best this week sorry again for the very long delay I really look forward merging such improvements

[codecov[bot]]

Codecov Report

Merging #2135 (e13c0e9) into master (b718c7d) will decrease coverage by 0.11%. The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #2135      +/-   ##
==========================================
- Coverage   56.80%   56.69%   -0.12%     
==========================================
  Files         138      138              
  Lines       17142    17142              
==========================================
- Hits         9737     9718      -19     
- Misses       7405     7424      +19     

Impacted Files	Coverage Δ
src/libsync/theme.cpp	12.07% <0.00%> (ø)
src/libsync/vfs/cfapi/hydrationjob.cpp	52.68% <0.00%> (-3.77%)	:arrow_down:
src/libsync/vfs/cfapi/vfs_cfapi.cpp	85.09% <0.00%> (-2.36%)	:arrow_down:
src/libsync/vfs/cfapi/cfapiwrapper.cpp	72.89% <0.00%> (-1.37%)	:arrow_down:
src/libsync/syncengine.cpp	86.65% <0.00%> (-0.55%)	:arrow_down:
src/libsync/discovery.cpp	84.86% <0.00%> (+0.29%)	:arrow_up:

[nextcloud-desktop-bot]

AppImage file: Nextcloud-PR-2135-e13c0e9fbcb1c7318a19fd1ddc0f884d26314b1c-x86_64.AppImage

To test this change/fix you can simply download above AppImage file and test it.

Please make sure to quit your existing Nextcloud app and backup your data.