Skip to content
This repository has been archived by the owner on Jul 28, 2021. It is now read-only.

feat(view): add 'view' command #27

Merged
merged 14 commits into from
Nov 13, 2018
Merged

feat(view): add 'view' command #27

merged 14 commits into from
Nov 13, 2018

Conversation

larsgw
Copy link
Contributor

@larsgw larsgw commented Nov 12, 2018

Move to use JSX and use register command.
Allow unlimited items when field is specified.
Add packument info to outputted JSON, as is custom in `npm view`. It's a 
bit double if multiple versions are queried, but that's just how it is. 
Also allows packument-specific props to be queried.
Allow dots (.) in props with a special prop syntax:

Data:
    { foo: {
      'bar.baz': 1,
      bar: { baz: 2 }
    } }

Query:
    foo[bar.baz] // -> 1

Useful to query versions, deps:

    time[0.10.1]
    dependencies[lodash.uniq]
Show dependencies in columns, matching the old behaviour.
Copy link
Contributor

@zkat zkat left a comment

Choose a reason for hiding this comment

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

holy shit this works so nice. Good job!

@zkat zkat merged commit 500207c into npm:latest Nov 13, 2018
Copy link

@legodude17 legodude17 left a comment

Choose a reason for hiding this comment

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

I'm so excited for this!

@@ -0,0 +1,51 @@
'use strict'

const npmlog = require('npmlog')

Choose a reason for hiding this comment

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

I'm pretty sure you could use libnpm.log.

)

if (!props && items.length > maxItems) {
items = items.slice(0, --maxItems).concat(

Choose a reason for hiding this comment

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

Why are you decrementing maxItems here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That should probably be done more explicitly, but basically: if the max amount of rows is 6, there should be 5 items and one line explaining there are more. I could also have increased the maxItems check by one, but both are wrong, and I found this one more practical, as maxItems now always limits the rows.

Choose a reason for hiding this comment

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

Is there a reason you use --maxItems and not maxItems - 1?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because it's referenced once more (again, not should have done that more explicitly).

builder (yargs) {
return yargs.help().alias('help', 'h').options(View.options)
},
options: Object.assign(require('../common-opts'), {}),

Choose a reason for hiding this comment

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

Wouldn't it make more sense if these arguments were reversed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, don't know why I did it that way... Not that it matters, at the moment.

@legodude17
Copy link

Whoops started that earlier today.

@larsgw
Copy link
Contributor Author

larsgw commented Nov 13, 2018

@legodude17 No problem, there might be a refactor (to use Ink 2) coming either way, so feedback is appreciated.

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

Successfully merging this pull request may close these issues.

3 participants