search

winapps-org / winapps

The winapps main project, forked from https://github.com/Fmstrat/winapps/
Other
998 stars 45 forks source link

Fix libvirt group detection and program calling #97

Closed LDprg closed 4 months ago

LDprg commented 4 months ago

Fixes for #95. Solves wrong invert when detecting the libvirt group. And fixed a wrong argumentwhen callibg freerdp.

LDprg commented 4 months ago

@oskardotglobal I did only write this on the go, so I couldn't test it.

jrevillard commented 4 months ago

Hello,

Do you finally tacle xfreerdp v2 or v3 ?.... You cannot support both simply as the syntax completely changed. In this MR, a move to v3 was done: https://github.com/winapps-org/winapps/pull/87 ... and here you go back to v2....

Best, Jerome

oskardotglobal commented 4 months ago

Hello,

Do you finally tacle xfreerdp v2 or v3 ?.... You cannot support both simply as the syntax completely changed. In this MR, a move to v3 was done: #87 ... and here you go back to v2....

Best, Jerome

Just a small issue, this is why we review PRs This of course will not be changed, we will be staying with freerdp3

LDprg commented 4 months ago

We might want to use a flag to switch between v2 and v3. Or we set v3 as official requirement, which is properly documented.

Edit: @oskardotglobal I did look up the documentation of v2 v3 (which is kind of horrible). Sure that app:program: is not basically the same as app:. As far as I understand app: the kinda depricated syntax but still valid.

LDprg commented 4 months ago

I included the libvirt fix in the refactoring pr #98. If it gets merged this is not needed

oskardotglobal commented 4 months ago

Then we can close this PR, right?

LDprg commented 4 months ago

This is taken over by #98