RDP: Warn if trying to share a non existing folder
ClosedPublic

Authored by aacid on Nov 30 2019, 11:09 PM.

Details

Summary

More xfreerdp just bails out if you try to share a non existing folder and you get a :o face without knowing what went wrong

Diff Detail

Repository
R436 KRDC
Branch
release/19.12
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 19320
Build 19338: arc lint + arc unit
aacid requested review of this revision.Nov 30 2019, 11:09 PM
aacid created this revision.
aacid updated this revision to Diff 70642.Nov 30 2019, 11:10 PM
aacid retitled this revision from RDO: Warn if trying to share a non existing folder to RDP: Warn if trying to share a non existing folder.

fix commit log

This is a i18n break but since if you have this problem you won't be able to figure out what the problem is unless you debug the source code i'd like for this to go in to 19.12

pino added a subscriber: pino.Nov 30 2019, 11:23 PM
pino added inline comments.
rdp/rdpview.cpp
120

Since this is a new string, please use the correct HIG style:

  • no contractions, so "don't" -> "do not", etc
  • Title Capitalization for the dialog title, so "Media Folder does not Exist"

Also a small nit: "Folder %1 does not exist." -> "The folder %1 does not exist."

aacid updated this revision to Diff 70644.Nov 30 2019, 11:37 PM

update strings according to Pino's comments

volkov added inline comments.Dec 2 2019, 11:21 AM
rdp/rdpview.cpp
119

Can a directory be not readable?

volkov added inline comments.Dec 2 2019, 11:40 AM
rdp/rdpview.cpp
119

Hmmm, QDir().exists(shareMediaPath) only checks for the existence of shareMediaPath, it doesn't check that it's a directory.
QDir(shareMediaPath).exists() should do the job.

aacid added a comment.Dec 2 2019, 11:06 PM

I'm going to land this *now* since i want the translatable message to be available for translators so they have a chance to translate it before thursday.

But if you have any other comment i'm happy to fix/address it.

rdp/rdpview.cpp
119

true, i totally failed at reading the docu

Though it's funny since it actually "works" (i.e. not fails to start that was my original problem) if you give it a file path too, but let's stick with dirs

119

I just double checked and it doesn't matter if the directory is readable or not (at least for my particular verison of xfreerdp)

This revision was not accepted when it landed; it landed in state Needs Review.Dec 2 2019, 11:06 PM
This revision was automatically updated to reflect the committed changes.