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

Extend and improve cordova info output #616

Merged
merged 11 commits into from
Aug 23, 2018
Merged

Conversation

raphinesse
Copy link
Contributor

What it does

Extend and improve cordova info output as well as a code cleanup of info.js.

Before
$ cordova info
Collecting Data...


Node version: v8.11.1

Cordova version: 8.0.1-dev

Config.xml file: 

<?xml version='1.0' encoding='utf-8'?>
<widget id="io.cordova.hellocordova" version="1.0.0" xmlns="http://www.w3.org/ns/widgets" xmlns:cdv="http://cordova.apache.org/ns/1.0">
    <name>HelloCordova</name>
    <description>
        A sample Apache Cordova application that responds to the deviceready event.
    </description>
    <author email="dev@cordova.apache.org" href="http://cordova.io">
        Apache Cordova Team
    </author>
    <content src="index.html" />
    <access origin="*" />
    <allow-intent href="http://*/*" />
    <allow-intent href="https://*/*" />
    <allow-intent href="tel:*" />
    <allow-intent href="sms:*" />
    <allow-intent href="mailto:*" />
    <allow-intent href="geo:*" />
    <platform name="android">
        <allow-intent href="market:*" />
    </platform>
    <platform name="ios">
        <allow-intent href="itms:*" />
        <allow-intent href="itms-apps:*" />
    </platform>
    <engine name="android" spec="^7.0.0" />
    <engine name="ios" spec="^4.5.4" />
    <plugin name="cordova-plugin-file" spec="^4.3.3" />
    <plugin name="cordova-plugin-file-transfer" spec="^1.6.3" />
    <plugin name="cordova-plugin-zip" spec="^3.1.0" />
</widget>


Plugins: 

code-push,cordova-plugin-code-push,cordova-plugin-compat,cordova-plugin-device,cordova-plugin-dialogs,cordova-plugin-file,cordova-plugin-file-transfer,cordova-plugin-whitelist,cordova-plugin-zip

Android platform:

*************************************************************************
The "android" command is deprecated.
For manual SDK, AVD, and project management, please use Android Studio.
For command-line tools, use tools/bin/sdkmanager and tools/bin/avdmanager
*************************************************************************
Running /opt/android-sdk/tools/bin/avdmanager list target

Available Android targets:==============] 100% Fetch remote repository...       
----------
id: 6 or "android-27"
     Name: Android API 27
     Type: Platform
     API level: 27
     Revision: 1

Error retrieving iOS platform information: Error: xcodebuild: Command failed with exit code ENOENT

After
$ cordova info
Cordova version 8.0.1-dev with:
  cordova-common@2.2.1
  cordova-create@1.1.2
  cordova-fetch@1.3.0
  cordova-js@4.2.2
  cordova-serve@2.0.0

Environment: 
  OS: linux
  Node: v8.11.1
  npm: 6.1.0

Plugins:
  code-push
  cordova-plugin-code-push
  cordova-plugin-compat
  cordova-plugin-device
  cordova-plugin-dialogs
  cordova-plugin-file
  cordova-plugin-file-transfer
  cordova-plugin-whitelist
  cordova-plugin-zip

Android platform:
  *************************************************************************
  The "android" command is deprecated.
  For manual SDK, AVD, and project management, please use Android Studio.
  For command-line tools, use tools/bin/sdkmanager and tools/bin/avdmanager
  *************************************************************************
  Running /opt/android-sdk/tools/bin/avdmanager list target

  Available Android targets:==============] 100% Fetch remote repository...       
   ----------
  id: 6 or "android-27"
       Name: Android API 27
       Type: Platform
       API level: 27
       Revision: 1

iOS platform:
  ERROR: xcodebuild: Command failed with exit code ENOENT

config.xml <<EOF
<?xml version='1.0' encoding='utf-8'?>
<widget id="io.cordova.hellocordova" version="1.0.0" xmlns="http://www.w3.org/ns/widgets" xmlns:cdv="http://cordova.apache.org/ns/1.0">
    <name>HelloCordova</name>
    <description>
        A sample Apache Cordova application that responds to the deviceready event.
    </description>
    <author email="dev@cordova.apache.org" href="http://cordova.io">
        Apache Cordova Team
    </author>
    <content src="index.html" />
    <access origin="*" />
    <allow-intent href="http://*/*" />
    <allow-intent href="https://*/*" />
    <allow-intent href="tel:*" />
    <allow-intent href="sms:*" />
    <allow-intent href="mailto:*" />
    <allow-intent href="geo:*" />
    <platform name="android">
        <allow-intent href="market:*" />
    </platform>
    <platform name="ios">
        <allow-intent href="itms:*" />
        <allow-intent href="itms-apps:*" />
    </platform>
    <engine name="android" spec="^7.0.0" />
    <engine name="ios" spec="^4.5.4" />
    <plugin name="cordova-plugin-file" spec="^4.3.3" />
    <plugin name="cordova-plugin-file-transfer" spec="^1.6.3" />
    <plugin name="cordova-plugin-zip" spec="^3.1.0" />
</widget>

EOF

package.json <<EOF
{
    "name": "helloworld",
    "displayName": "HelloCordova",
    "version": "1.0.0",
    "description": "A sample Apache Cordova application that responds to the deviceready event.",
    "main": "index.js",
    "scripts": {},
    "author": "Apache Cordova Team",
    "license": "Apache-2.0",
    "dependencies": {
        "code-push": "^2.0.4",
        "cordova": "^8.0.0",
        "cordova-android": "^7.0.0",
        "cordova-ios": "^4.5.4",
        "cordova-plugin-code-push": "^1.11.2",
        "cordova-plugin-compat": "^1.2.0",
        "cordova-plugin-device": "^2.0.1",
        "cordova-plugin-file": "^4.3.3",
        "cordova-plugin-file-transfer": "^1.6.3",
        "cordova-plugin-whitelist": "^1.3.3",
        "cordova-plugin-zip": "^3.1.0"
    },
    "cordova": {
        "platforms": [
            "android",
            "ios"
        ],
        "plugins": {
            "cordova-plugin-device": {},
            "cordova-plugin-code-push": {},
            "cordova-plugin-whitelist": {},
            "cordova-plugin-file": {},
            "cordova-plugin-file-transfer": {},
            "cordova-plugin-zip": {}
        }
    }
}
EOF

What testing has been done on this change?

Ran cordova info in various settings.

This only clutters users' projects. If a user wants the output in a
file, they can simply redirect stdout.
* Output files last & delimit them in HEREDOC style
* Indent all regular blocks
* Remove empty trailing lines
* Code cleanup
This contains
* OS (process.platform)
* Node version
* npm version
.filter(name => name.startsWith('cordova-'))
.map(name => `${name}@${versionFor(name)}`)
.join('\n');
return `Cordova version ${pkg.version} with:\n${indent(deps)}`;
Copy link
Member

Choose a reason for hiding this comment

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

=> Cordova CLI version ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, actually it's the cordova-lib version. CLI is should really only be a thin wrapper around lib.

Copy link
Member

Choose a reason for hiding this comment

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

Oh. Ohhhh.

Well, that's too complicated for me ;)

How about outputting the CLI version in the "headline" and adding cordova-lib in the list of "packages" then? That would be what some user (read: me) expects and still have all the information you need.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IIRC, that would be difficult with the current approach (we are looking at our dependencies, CLI depends on us though).

This should definitely go onto our TODO list though.

Copy link
Member

Choose a reason for hiding this comment

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

Now an issue at: #659

Copy link
Member

Choose a reason for hiding this comment

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

Change it to Cordova lib version here in the meantime to avoid confusion?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

}

function listPlugins (projectRoot) {
var pluginPath = path.join(projectRoot, 'plugins');
var plugins = cordova_util.findPlugins(pluginPath);
var plugins = cordova_util.findPlugins(pluginPath).join('\n');
return 'Plugins:' + (plugins.length ? '\n' + indent(plugins) : ' []');
Copy link
Member

Choose a reason for hiding this comment

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

Could this also output the plugin version?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure. We would have to check.

Copy link
Member

Choose a reason for hiding this comment

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

Now tracked at #660

Copy link
Member

@janpio janpio left a comment

Choose a reason for hiding this comment

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

Oh very nice!

Could you maybe add "Platforms:" section similar to "Plugins" or "Environment" (with version info) before the platform details (getPlatformInfo) are output?

(And see other comments in code directly)

@raphinesse
Copy link
Contributor Author

@janpio Would you agree to merge my current version and collect further improvements in an issue for them to be implemented in later PRs?

That way, we can have some improvement now and polish it later (I have no time to do so, right now)

@janpio
Copy link
Member

janpio commented Aug 23, 2018

Ok, will do the Cordova => Cordova lib change myself and extract the other stuff into issues, then merge.

@raphinesse
Copy link
Contributor Author

@janpio Great, thanks!

@janpio
Copy link
Member

janpio commented Aug 23, 2018

Platform information request now tracked in #661

@janpio janpio merged commit 3124881 into apache:master Aug 23, 2018
@raphinesse raphinesse deleted the info branch August 23, 2018 14:08
@raphinesse raphinesse removed the request for review from dpogue August 23, 2018 14:08
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.

None yet

2 participants