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/cli: Add show discovery command (fixes #8007) #8378

Merged
merged 2 commits into from Jul 13, 2022

Conversation

sec65
Copy link
Contributor

@sec65 sec65 commented Jun 7, 2022

Purpose

Add show disovery command to cli (fixes #8007)

Testing

Execute syncthing cli show discovery on the command line

Documentation

Since the other show commands are not described in the documentation as far as I have seen, I don't think a change is necessary.

Authorship

Your name and email will be added automatically to the AUTHORS file
based on the commit metadata.

@imsodin imsodin changed the title cli: Add show disovery command (fixes #8007) cli: Add show discovery command (fixes #8007) Jun 7, 2022
@@ -35,6 +35,11 @@ var showCommand = cli.Command{
Usage: "Report about connections to other devices",
Action: expects(0, indexDumpOutput("system/connections")),
},
{
Name: "discovery",
Usage: "Report about the local discovery cache",
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for your contribution!

I know the docs contain the same sentence, however with (supposedly) more user interactions with this command it would be nice if it was a bit more descriptive. E.g. "Discovered addresses of remote devices (from cache of the running syncthing instance)". The cache bit is relevant, such that no-one thinks it looks it up actively e.g. from global discovery servers due to this method - "of the running syncthing instance" may be overly verbouse and not add anything, not sure - anyway all just a suggestions.

{
Name: "discovery",
Usage: "Report about the local discovery cache",
Action: expects(0, indexDumpOutput("system/discovery")),
Copy link
Member

Choose a reason for hiding this comment

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

That's definitely a misnomer by now - prettyPrintOutput or something would be more appropriate. Not something needed to be addressed here.

@sec65
Copy link
Contributor Author

sec65 commented Jun 7, 2022

@imsodin Thanks for your response. I changed the description to your proposal.

@acolomb
Copy link
Member

acolomb commented Jul 13, 2022

Would love to see this happen. Just now I had been looking for exactly that command again without success.

@imsodin I think your review comments were addressed?

@imsodin imsodin changed the title cli: Add show discovery command (fixes #8007) cmd/syncthing/cli: Add show discovery command (fixes #8007) Jul 13, 2022
@imsodin imsodin merged commit 4335285 into syncthing:main Jul 13, 2022
calmh added a commit to calmh/syncthing that referenced this pull request Jul 21, 2022
* main:
  gui, man, authors: Update docs, translations, and contributors
  cmd/syncthing/cli: Add show discovery command (fixes syncthing#8007) (syncthing#8378)
  gui, man, authors: Update docs, translations, and contributors
  lib/osutil: Only announce address of interfaces which are up (fixes syncthing#7458) (syncthing#8422)
  gui: Fix missing span end tag and missing nbsp semicolon in HTML (syncthing#8419)
@calmh calmh added this to the v1.20.4 milestone Jul 26, 2022
calmh added a commit to calmh/syncthing that referenced this pull request Jul 28, 2022
* main:
  cmd/syncthing, lib/config: Remove restartOnWakeup option & functionality (fixes syncthing#8448) (syncthing#8449)
  gui: Remove blank meta tags (syncthing#8362)
  gui: Add device sync status (fixes syncthing#7981) (syncthing#8401)
  gui: Fix detailed staggered versioning information in folder info (ref syncthing#8348) (syncthing#8433)
  all: Support syncing ownership (fixes syncthing#1329) (syncthing#8434)
  gui, man, authors: Update docs, translations, and contributors
  lib/model, lib/config: Apply sensible defaults for auto-accepted encrypted folder (fixes syncthing#8296) (syncthing#8427)
  gui: Move filesystem watcher explanation from tooltip to help block (syncthing#8432)
  gui: Use discovered IDs from cache when adding a new remote device (syncthing#8382)
  build: Update goleveldb (syncthing#8440)
  gui, man, authors: Update docs, translations, and contributors
  cmd/syncthing/cli: Add show discovery command (fixes syncthing#8007) (syncthing#8378)
  gui, man, authors: Update docs, translations, and contributors
  lib/osutil: Only announce address of interfaces which are up (fixes syncthing#7458) (syncthing#8422)
  gui: Fix missing span end tag and missing nbsp semicolon in HTML (syncthing#8419)
@st-review st-review added the frozen-due-to-age Issues closed and untouched for a long time, together with being locked for discussion label Jul 14, 2023
@syncthing syncthing locked and limited conversation to collaborators Jul 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
frozen-due-to-age Issues closed and untouched for a long time, together with being locked for discussion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support listing the discovered devices using the cli
5 participants