Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wayvncctl #171

Merged
merged 11 commits into from
Nov 10, 2022
Merged

wayvncctl #171

merged 11 commits into from
Nov 10, 2022

Conversation

lack
Copy link
Collaborator

@lack lack commented Oct 31, 2022

  • Refactor pointer initialization code
  • Refactor output selection code
  • Add output_cycle to get next/prev outputs
  • Add functions to switch outputs on the fly
  • Switch to previous output if current output disappears
  • Add json-ipc message plumbing
  • Add ctl control socket and initial command infrastructure
  • Refactor some common utilities out of main
  • Add ctl-client code
  • Add initial wayvncctl executable
  • Add wayvnctl help command
  • Add wayvncctl version command
  • Manpage updates for wayvnc and wayvncctl

I have read and understood CONTRIBUTING.md and its associated documents.

@lack lack force-pushed the wayvncctl branch 4 times, most recently from 76a4a08 to 63858a1 Compare October 31, 2022 10:56
Copy link
Owner

@any1 any1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The quality here is well above average when it comes to code I've reviewed for this project.

I did not have time today to look at this as thoroughly as I would have liked and I may have missed important details.

Overall, I would like to see the log level dropped a little bit. There is an nvnc_trace() macro which I feel is underutilised.

src/json-ipc.c Outdated Show resolved Hide resolved
src/json-ipc.c Outdated Show resolved Hide resolved
src/ctl-server.c Outdated Show resolved Hide resolved
src/ctl-server.c Outdated Show resolved Hide resolved
src/main.c Outdated Show resolved Hide resolved
src/ctl-client.c Outdated Show resolved Hide resolved
src/ctl-client.c Outdated Show resolved Hide resolved
src/wayvncctl.c Outdated Show resolved Hide resolved
meson.build Outdated Show resolved Hide resolved
wayvncctl.scd Outdated Show resolved Hide resolved
@lack lack force-pushed the wayvncctl branch 13 times, most recently from 2bf3ddb to b6dbc84 Compare November 6, 2022 07:50
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
include/ctl-server.h Outdated Show resolved Hide resolved
src/ctl-client.c Outdated Show resolved Hide resolved
src/ctl-client.c Outdated Show resolved Hide resolved
src/ctl-server.c Outdated Show resolved Hide resolved
src/ctl-server.c Outdated Show resolved Hide resolved
src/ctl-server.c Outdated Show resolved Hide resolved
wayvncctl.scd Outdated Show resolved Hide resolved
src/util.c Show resolved Hide resolved
@lack lack force-pushed the wayvncctl branch 6 times, most recently from ceba956 to 12707d8 Compare November 8, 2022 06:50
Copy link
Owner

@any1 any1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost ready!

README.md Show resolved Hide resolved
src/ctl-client.c Outdated Show resolved Hide resolved
src/ctl-client.c Outdated Show resolved Hide resolved
src/ctl-client.c Outdated Show resolved Hide resolved
src/ctl-client.c Show resolved Hide resolved
src/ctl-server.c Outdated Show resolved Hide resolved
src/ctl-server.c Show resolved Hide resolved
src/ctl-server.c Show resolved Hide resolved
src/ctl-server.c Outdated Show resolved Hide resolved
src/wayvncctl.c Outdated Show resolved Hide resolved
@lack lack force-pushed the wayvncctl branch 4 times, most recently from 373825e to 8c13cc5 Compare November 9, 2022 06:27
@lack
Copy link
Collaborator Author

lack commented Nov 9, 2022

Okay! Tested most code paths with valgrind and fixed some refcount-based leaks.

@any1 : ready for another review!

Copy link
Owner

@any1 any1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM after a few final tweaks.

src/util.c Show resolved Hide resolved
src/ctl-server.c Outdated Show resolved Hide resolved
src/ctl-client.c Outdated Show resolved Hide resolved
src/ctl-server.c Outdated Show resolved Hide resolved
lack added 11 commits November 9, 2022 23:27
This implements the first wayvncctl command: set-output

Signed-off-by: Jim Ramsay <[email protected]>
Signed-off-by: Jim Ramsay <[email protected]>
Signed-off-by: Jim Ramsay <[email protected]>
Also adds the '--json' option to produce machine-readable output.

Signed-off-by: Jim Ramsay <[email protected]>
@lack
Copy link
Collaborator Author

lack commented Nov 10, 2022

@any1 Done!

Also fixed a handful of char *foo => char* foo we both missed :)

@any1 any1 merged commit 231a08c into any1:master Nov 10, 2022
@any1
Copy link
Owner

any1 commented Nov 10, 2022

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants