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

reinstate --help #652

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

reinstate --help #652

wants to merge 2 commits into from

Conversation

dominictarr
Copy link
Contributor

pretty much a complete rewrite of the internal help system.

now with muxrpcli@3 has each plugin own it's own help - plugins should have a help method that returns a data structure describing all commands and all their arguments. That has been implemented for ssb-db and ssb-gossip so far, but all the plugins will need that.

This data is also machine readable, which I think will lend to other useful things, such as validation of inputs. Also since it describes the patterns, it would be usable for other types of interface, http, cli or javascript.

@dominictarr
Copy link
Contributor Author

dominictarr commented Apr 1, 2019

modules that need "help"

  • ssb-replicate
  • ssb-friends
  • ssb-blobs
  • ssb-invite
  • ssb-peer-invites
  • ssb-query
  • ssb-ooo
  • ssb-device-address
  • ssb-search
  • ssb-identities
  • ssb-plugins

@staltz
Copy link
Member

staltz commented Apr 1, 2019

Could the help data structure be described somewhere as a convention? It smells like a module in itself (similar to multiserver-address).

@dominictarr
Copy link
Contributor Author

@staltz yes, it's documented here: https://github.com/dominictarr/muxrpc-usage#data-structure

@dominictarr
Copy link
Contributor Author

none of the plugins moved in https://github.com/ssbc/ssb-server/pull/664/files have commands, so merging that doesn't block this. Except ssb-plugins, which needs to be merged: #653

Copy link
Contributor

@ahdinosaur ahdinosaur left a comment

Choose a reason for hiding this comment

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

noticed this was "Waiting for Review", so left some feedback! 🗒️ 😺

//get config as cli options after --, options before that are
//options to the command.

module.exports = function (config, argv) {
Copy link
Contributor

Choose a reason for hiding this comment

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

can we name this function cli or something?

as down at the bottom, we call module.exports(...) and took me a few moments to grok what was happening.

var i = argv.indexOf('--')
var conf = argv.slice(i+1)
argv = ~i ? argv.slice(0, i) : argv
module.exports(Config(process.env.ssb_appname, minimist(conf)), argv)
Copy link
Contributor

Choose a reason for hiding this comment

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

there's a lot happening here, but it's hard to tell what is happening, what about:

var args = process.argv.slice(2)
var [methodArgs, configArgs] = splitArgsBySeparator(args)
var appName = process.env.ssb_appname
var configOptions = minimist(configArgs)
var config = Config(appName, configOptions)

cli(config, methodArgs)

function splitArgvBySeparator (args) {
  var splitIndex = args.indexOf('--')
  var methodArgs = splitIndex == -1 ? args : args.slice(0, splitIndex)
  var configArgs = args.slice(splitIndex + 1)
  return [methodArgs, configArgs]
} 

})
}

if(!module.parent && process.title != 'browser') {
Copy link
Contributor

Choose a reason for hiding this comment

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

can there be a comment to describe what this is doing?

@dominictarr
Copy link
Contributor Author

@corlock had a problem that, running ssb-server inside of patchwork, couldn't use --help. I think whats happening is that --help is really, new, so that i'm not sure it's in the versions of modules that patchwork has?

@stale
Copy link

stale bot commented Aug 28, 2019

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

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

Successfully merging this pull request may close these issues.

3 participants