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

[RFC] Multiple Cabals Continuation #89

Merged
merged 45 commits into from
Nov 16, 2018
Merged

Conversation

cblgh
Copy link
Member

@cblgh cblgh commented Nov 6, 2018

this is a continuation of nick's pr #84

i fucked up my branches so here we are with a new pr

nikolaiwarner and others added 23 commits October 27, 2018 20:57
Co-Authored-By: nikolaiwarner <nikolaiwarner@users.noreply.github.com>
yaay it's a lot shorter, and kind of makes sense if we have
`cabal-client` as a module
the users object, returned from `cabal-core/views/users.js:getAll()`
is empty at first start (nothing has been recorded in the kappa-core db
yet), so the iteration in updateLocalKey will never occur. thus we *at
least* set the key we've gotten (since we will always know that) and
then try to find the name as registered in the database
@cblgh cblgh mentioned this pull request Nov 6, 2018
commands.js Outdated Show resolved Hide resolved
@hackergrrl
Copy link
Member

hackergrrl commented Nov 8, 2018 via email


var cabal = Cabal(args.db, args.key)
if (args.new) {
var key = crypto.keyPair().publicKey.toString('hex')
Copy link
Member

Choose a reason for hiding this comment

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

👌

package.json Outdated
@@ -1,17 +1,20 @@
{
"name": "cabal",
"version": "5.0.0",
"version": "5.0.1",
Copy link
Member

Choose a reason for hiding this comment

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

Can we do version bump in a separate PR?

@cblgh
Copy link
Member Author

cblgh commented Nov 14, 2018

after some discussions in #cabal-club we decided that it's better to get this into master and keep the show going

we still have bugs with multi-cabal, but the config stuff is really nice, so i'm going to try to put multi-cabal behind an --experimental flag and fix the bugs after the fact. i don't think affect single-cabal usage

tasks

  • create --experimental cli flag to enable multi-cabal
  • add alias section to config so people can do cabal --key cabal://00...ff --alias malmo-ppl; cabal --join malmo-ppl)
  • revert version bump and do it after pr has been merged into master
  • rename protocolVersion to databaseVersion
  • add --aliases to print aliases
  • add --clear to remove all aliases
  • add --forget to forget a specified alias
  • make cabal's config handling more robust
  • make cabal join the most recently joined/created cabal if run without options e.g. cabal
  • merge into master
  • bump master version
  • publish new cabal-core to npm
  • publish cli to npm

i also added --join to join a cabal while disregarding the config (not sure about that one, but i felt the need for it w/ multi)
closes #70
to keep going we'll hide multi-cabal behind a flag, and keep on
working on its bugs in secret :3
@cblgh
Copy link
Member Author

cblgh commented Nov 14, 2018

we're now printing saved aliases

this allows us a cabalist to print out which saved cabal aliases they
have. a good memory aid for the chatting individual

part of #70
--clear clears all aliases
--forget forgets the specified alias
also:
* improve cabal's config handling and make it more robust
Copy link
Member

@nikolaiwarner nikolaiwarner left a comment

Choose a reason for hiding this comment

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

👍 this is 🔥🔥🔥🔥🔥

@@ -84,165 +95,246 @@ function NeatScreen (cabal) {
else return
this.bus.emit('render')
})

// move between window panes with ctrl+j
this.neat.input.on('alt-n', () => {
Copy link
Member

Choose a reason for hiding this comment

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

alt-n still doesnt work for me on mac terminal ... but we can solve it after this lands

Copy link
Member

@hackergrrl hackergrrl left a comment

Choose a reason for hiding this comment

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

I noticed you're re-introducing package-lock.json -- intentional? LGTM otherwise!

@cblgh
Copy link
Member Author

cblgh commented Nov 16, 2018

@noffle: I noticed you're re-introducing package-lock.json -- intentional? LGTM otherwise!

that lil sucker just decided to appear, not intended!

@cblgh cblgh merged commit cf322c0 into master Nov 16, 2018
@joehand joehand deleted the multi-cabal-protocol-version branch July 9, 2019 17:16
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