Gorkem, I also found an old JIRA duplicate issue for this which had listed an old suggestion I think is interesting: https://issues.apache.org/jira/browse/CB-4624
Specifically, instead of introducing save/restore commands, we could mirror "npm install" and its use of "--save": - npm install -> cordova plugin add - restores deps - npm install foo -> cordova plugin add foo - install foo, but don't add it to deps - npm install foo --save -> cordova plugin add foo --save - install foo, and do add it to deps - npm install --save -> cordova plugin add --save - don't install anything, just save the current installed plugins into deps Just something to consider. (note, just tried it again and npm install --save doesn't actually do what I want.. wonder if its supported..) -Michal On Wed, Apr 23, 2014 at 11:39 PM, Michal Mocny <mmo...@chromium.org> wrote: > Gorkem, as an orthogonal issue to the syntax we use, I think there is > another small problem with this patch as-is. > > "cordova plugin add org.apache.cordova.file-transfer && cordova plugin > save" would have my application explicitly depend on > org.apache.cordova.file. If in the future the dependency is > removed/moved/renamed, my app would explicitly try to install it when > running "cordova plugin restore". > > As a first version I think this is acceptable, but I think we may want a > better solution eventually. > > > On Tue, Apr 22, 2014 at 1:18 PM, Michal Mocny <mmo...@chromium.org> wrote: > >> >> >> >> On Tue, Apr 22, 2014 at 11:37 AM, Gorkem Ercan <gorkem.er...@gmail.com>wrote: >> >>> On Fri, Apr 18, 2014 at 7:21 PM, Michal Mocny <mmo...@chromium.org> >>> wrote: >>> >>> > On Fri, Apr 18, 2014 at 4:11 PM, Gorkem Ercan <gorkem.er...@gmail.com >>> > >wrote: >>> > >>> > > On Thu, Apr 17, 2014 at 3:46 PM, Michal Mocny <mmo...@chromium.org> >>> > wrote: >>> > > >>> > > > Took a quick glance. General questions: >>> > > > - why the need for save? Why not just alter the list on each >>> cordova >>> > > > plugin add/rm? >>> > > > >>> > > >>> > > I do not want to force this workflow. Calling save does not bring >>> much >>> > > overhead, considering plugin list does not probably change >>> constantly. >>> > > >>> > >>> > If you are going to make this choice, then I would not like to >>> > automatically install on prepare. There should be an explicit "load" >>> > command. (those names are wrong, but you get the point). >>> > >>> > Either we automatically manage plugin installs, or explicitly manage >>> them. >>> > I'm happy to start with explicit and support opt-in to automatic >>> handling >>> > later once we have confidence in the feature. >>> > >>> >>> Makes sense... adding a 'restore' command and removing from prepare. We >>> can >>> add an auto-restore config in the next iteration. >>> >> >> Excellent, thanks. >> >> >>> >>> >>> > >>> > >>> > > >>> > > >>> > > > - wouldn't "cordova plugin rm foo && cordova prepare" re-install >>> that >>> > > > plugin right now? >>> > > > >>> > > >>> > > This workflow would require an explicit update to config.xml if a >>> plugin >>> > is >>> > > persisted in there. This is a good point, I shall update plugins rm >>> to >>> > > print a warning about it. >>> > >>> > >>> > > >>> > > > - why the name <feature> and not <dependency> ? I think this >>> > > functionality >>> > > > should overlap with the plugin.xml spec. >>> > > > >>> > > > >>> > > feature tag lives in the w3c widget spec which has advantages for >>> those >>> > > (like myself) who does provide tools for editing config,xml. >>> > > >>> > >>> > We are no longer using that spec, and I as we move more functionality >>> from >>> > plugins.xml into config.xml we should strive to keep them in line. It >>> also >>> > makes our docs easier. >>> > >>> > >>> Tools developers are people too :) I think plugin.xml and config.xml has >>> two different audiences, I think there is a comparatively small >>> intersection of developers who are interested on both. At the moment, we >>> are pretty much within the w3c spec with the top-level config.xml and I >>> do >>> not see the benefit of breaking it. >>> >> >> Actually, I am thinking about tools devs. Namely, our tools devs who >> should be using the same config parsing and handlers for dependency >> management, etc. >> >> Anyway. You are putting in the sweat and blood on this feature, so you >> get double the voting power on this as far as I'm concerned. Still, I >> think we should bring this to the attention of the list to see how everyone >> feels. Sound fair? I'll start a quick thread. >> >> >>> >>> >>> > >>> > > >>> > > >>> > > >>> > > > I haven't put the PR through testing yet. >>> > > > >>> > > > >>> > > > On Thu, Apr 17, 2014 at 5:54 PM, Jesse <purplecabb...@gmail.com> >>> > wrote: >>> > > > >>> > > > > Yeah, that looks weird. >>> > > > > >>> > > > > @purplecabbage >>> > > > > risingj.com >>> > > > > >>> > > > > >>> > > > > On Thu, Apr 17, 2014 at 2:46 PM, kamrik <g...@git.apache.org> >>> wrote: >>> > > > > >>> > > > > > Github user kamrik commented on a diff in the pull request: >>> > > > > > >>> > > > > > >>> > > > >>> https://github.com/apache/cordova-cli/pull/165#discussion_r11755812 >>> > > > > > >>> > > > > > --- Diff: src/save.js --- >>> > > > > > @@ -0,0 +1,71 @@ >>> > > > > > +/** >>> > > > > > + Licensed to the Apache Software Foundation (ASF) >>> under one >>> > > > > > + or more contributor license agreements. See the >>> NOTICE >>> > file >>> > > > > > + distributed with this work for additional information >>> > > > > > + regarding copyright ownership. The ASF licenses this >>> file >>> > > > > > + to you under the Apache License, Version 2.0 (the >>> > > > > > + "License"); you may not use this file except in >>> compliance >>> > > > > > + with the License. You may obtain a copy of the >>> License at >>> > > > > > + >>> > > > > > + http://www.apache.org/licenses/LICENSE-2.0 >>> > > > > > + >>> > > > > > + Unless required by applicable law or agreed to in >>> writing, >>> > > > > > + software distributed under the License is distributed >>> on >>> > an >>> > > > > > + "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY >>> > > > > > + KIND, either express or implied. See the License for >>> the >>> > > > > > + specific language governing permissions and >>> limitations >>> > > > > > + under the License. >>> > > > > > +*/ >>> > > > > > + >>> > > > > > +var cordova_util = require('./util'), >>> > > > > > + ConfigParser = require('./ConfigParser'), >>> > > > > > + path = require('path'), >>> > > > > > + xml =require('./xml-helpers') >>> > > > > > + Q = require('q'), >>> > > > > > + events = require('./events'); >>> > > > > > + >>> > > > > > +; >>> > > > > > + >>> > > > > > + >>> > > > > > +module.exports = function save(target){ >>> > > > > > + var projectHome = cordova_util.cdProjectRoot(); >>> > > > > > + var configPath = >>> cordova_util.projectConfig(projectHome); >>> > > > > > + var configXml = new ConfigParser(configPath); >>> > > > > > + var pluginsPath = path.join(projectHome, 'plugins'); >>> > > > > > + var plugins = cordova_util.findPlugins(pluginsPath); >>> > > > > > + >>> > > > > > + return Q.all(plugins.map(function(plugin){ >>> > > > > > + var currentPluginPath = path.join(pluginsPath,plugin); >>> > > > > > + var name = readPluginName(currentPluginPath); >>> > > > > > + var id = plugin; >>> > > > > > + var version = readPluginVersion(currentPluginPath); >>> > > > > > + var features = configXml.doc.findall('feature'); >>> > > > > > + for(var i=0; i<features.length; i++){ >>> > > > > > + if(features[i].attrib.name === name){ >>> > > > > > + events.emit('results', 'An entry for "'+ >>> plugin+ '" >>> > > > > already >>> > > > > > exists'); >>> > > > > > + return Q(); >>> > > > > > + } >>> > > > > > + } >>> > > > > > + configXml.addFeature(name, JSON.parse('[{"name":"id", >>> > > > > > "value":"'+id+'"},{"name":"version", >>> "value":"'+version+'"}]')); >>> > > > > > --- End diff -- >>> > > > > > >>> > > > > > I might be missing something, but why JSON.parse() rather >>> than >>> > > just >>> > > > > > literal array of objects? >>> > > > > > >>> > > > > > >>> > > > > > --- >>> > > > > > If your project is set up for it, you can reply to this email >>> and >>> > > have >>> > > > > your >>> > > > > > reply appear on GitHub as well. If your project does not have >>> this >>> > > > > feature >>> > > > > > enabled and wishes so, or if the feature is enabled but not >>> > working, >>> > > > > please >>> > > > > > contact infrastructure at infrastruct...@apache.org or file a >>> JIRA >>> > > > > ticket >>> > > > > > with INFRA. >>> > > > > > --- >>> > > > > > >>> > > > > >>> > > > >>> > > >>> > >>> >> >> >
