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

cmd/syncthing: Better cli stdin handling (ref #9166) #9281

Merged
merged 1 commit into from Dec 11, 2023

Conversation

calmh
Copy link
Member

@calmh calmh commented Dec 11, 2023

Seems to work for me, @AudriusButkevicius.

@calmh calmh merged commit c53a1f2 into syncthing:main Dec 11, 2023
21 checks passed
@AudriusButkevicius
Copy link
Member

AudriusButkevicius commented Dec 11, 2023

There is still an issue however that the urfave cli instance is re-created on every line, which I think ends up doing a get/post for every line, but hey, atleast we're not execing.

@calmh
Copy link
Member Author

calmh commented Dec 11, 2023

Hmmm are you sure that wasn't the case before as well? It seems like it would require some machinery to avoid, with having a separate commit step before exiting after modifications and whatnot...

@AudriusButkevicius
Copy link
Member

AudriusButkevicius commented Dec 11, 2023

Ok, I'm making this shit up.

It's simply this:

func (h *configHandler) configAfter(_ *cli.Context) error {

Given the API is reflective, we don't know if the action is mutating or not, so it just had magic to avoid posting when you did a get etc. So that is still in place, and this behaviour is equivalent.

@calmh calmh added this to the v1.27.2 milestone Dec 12, 2023
calmh added a commit to calmh/syncthing that referenced this pull request Dec 16, 2023
* main: (89 commits)
  build: Update quic-go (fixes syncthing#9287)
  lib/model: Only handle relevant folder summaries (kqueue) (fixes syncthing#9183) (syncthing#9288)
  lib/model: Use a single lock (phase two: cleanup) (syncthing#9276)
  build(deps): bump actions/setup-go from 4 to 5 (syncthing#9279)
  lib/model: Use a single lock (syncthing#9275)
  cmd/syncthing: Better cli stdin handling (ref syncthing#9166) (syncthing#9281)
  cmd/syncthing: Mostly replace urfave/cli command line parser with alecthomas/kong (syncthing#9166)
  lib/nat: Fix test build failure (ref syncthing#9010)
  lib/model: Add pmut locking for DeviceStatistics (fixes syncthing#9274)
  lib/model: Add pmut locking for DeviceStatistics (fixes syncthing#9274)
  lib/model: Remove spurious "replacing service" failure event (ref syncthing#9271)
  lib/model: Remove spurious "replacing service" failure event (ref syncthing#9271)
  lib/nat, lib/upnp: IPv6 UPnP support (syncthing#9010)
  gui, man, authors: Update docs, translations, and contributors
  gui: Show folder/device status on small screens (syncthing#8643)
  lib/model: Remove runner during folder cleanup (fixes syncthing#9269) (syncthing#9271)
  build: Update dependencies (syncthing#9265)
  build: Revert specifics for Go 1.21.4, build using Go 1.21.5 (syncthing#9264)
  lib/fs: Reduce memory usage in xattrs handling (syncthing#9251)
  lib/model: Improve LastSeen handling (syncthing#9256)
  ...
calmh added a commit to calmh/syncthing that referenced this pull request Jan 14, 2024
* main:
  lib/model: Use a single lock (phase two: cleanup) (syncthing#9276)
  build(deps): bump actions/setup-go from 4 to 5 (syncthing#9279)
  lib/model: Use a single lock (syncthing#9275)
  cmd/syncthing: Better cli stdin handling (ref syncthing#9166) (syncthing#9281)
  cmd/syncthing: Mostly replace urfave/cli command line parser with alecthomas/kong (syncthing#9166)
  lib/nat: Fix test build failure (ref syncthing#9010)
  lib/model: Add pmut locking for DeviceStatistics (fixes syncthing#9274)
  lib/model: Add pmut locking for DeviceStatistics (fixes syncthing#9274)
  lib/model: Remove spurious "replacing service" failure event (ref syncthing#9271)
  lib/model: Remove spurious "replacing service" failure event (ref syncthing#9271)
  lib/nat, lib/upnp: IPv6 UPnP support (syncthing#9010)
  gui, man, authors: Update docs, translations, and contributors
  gui: Show folder/device status on small screens (syncthing#8643)
  lib/model: Remove runner during folder cleanup (fixes syncthing#9269) (syncthing#9271)
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.

None yet

2 participants