Return-Path: 
 <dev-return-32178-apmail-cordova-dev-archive=cordova.apache.org@cordova.apache.org>
X-Original-To: apmail-cordova-dev-archive@www.apache.org
Delivered-To: apmail-cordova-dev-archive@www.apache.org
Received: from mail.apache.org (hermes.apache.org [140.211.11.3])
	by minotaur.apache.org (Postfix) with SMTP id F300011F88
	for <apmail-cordova-dev-archive@www.apache.org>;
 Fri, 18 Apr 2014 20:11:53 +0000 (UTC)
Received: (qmail 34130 invoked by uid 500); 18 Apr 2014 20:11:53 -0000
Delivered-To: apmail-cordova-dev-archive@cordova.apache.org
Received: (qmail 34096 invoked by uid 500); 18 Apr 2014 20:11:53 -0000
Mailing-List: contact dev-help@cordova.apache.org; run by ezmlm
Precedence: bulk
List-Help: <mailto:dev-help@cordova.apache.org>
List-Unsubscribe: <mailto:dev-unsubscribe@cordova.apache.org>
List-Post: <mailto:dev@cordova.apache.org>
List-Id: <dev.cordova.apache.org>
Reply-To: dev@cordova.apache.org
Delivered-To: mailing list dev@cordova.apache.org
Received: (qmail 34080 invoked by uid 99); 18 Apr 2014 20:11:52 -0000
Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136)
    by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 18 Apr 2014 20:11:52 +0000
X-ASF-Spam-Status: No, hits=1.5 required=5.0
	tests=HTML_MESSAGE,RCVD_IN_DNSWL_LOW,SPF_PASS
X-Spam-Check-By: apache.org
Received-SPF: pass (athena.apache.org: domain of gorkem.ercan@gmail.com
 designates 209.85.160.175 as permitted sender)
Received: from [209.85.160.175] (HELO mail-yk0-f175.google.com)
 (209.85.160.175)
    by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 18 Apr 2014 20:11:49 +0000
Received: by mail-yk0-f175.google.com with SMTP id 131so1675376ykp.20
        for <dev@cordova.apache.org>; Fri, 18 Apr 2014 13:11:28 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
        d=gmail.com; s=20120113;
        h=mime-version:in-reply-to:references:date:message-id:subject:from:to
         :content-type;
        bh=awOI/eo+uY0QWEAJDak68WtNY8Sedu9jebpFKBZJbD0=;
        b=PrTpTbiiyLDscozTK7CPoJmjEtGFWlH3VcyLosT2M7ERZeeHEB7yqzXPeTWfqPfEOj
         nFeTGPkDACfdUpPB8kSYjdSk9/LImvU64ztyo26+dRSVHnoMF0HY6ll/XWYAArGR+8rc
         diklTV4fMf6WsoGrLPC1254RwiL8o1qvuknM8FeV6MnEel0TrGZNHoyYU4gHC1Yy80z6
         58Z66IAMAuIqruq3DmR112N6WsOUb457xh361XQPcZ959cCYIyio15kYL2q92WXHmlXw
         VLfAf4P8SSkFIy/WR3wRLOaDrQ8MCTPzO4XIunjUx4voyqR25JKpORDYv4MzKzcGRbum
         A7Fg==
MIME-Version: 1.0
X-Received: by 10.236.46.5 with SMTP id q5mr32303428yhb.21.1397851888237; Fri,
 18 Apr 2014 13:11:28 -0700 (PDT)
Received: by 10.170.85.66 with HTTP; Fri, 18 Apr 2014 13:11:28 -0700 (PDT)
In-Reply-To: 
 <CAEeF2Tc=mc5cPGJ_bTYzhuUW7RvWLEQOE2hTH_p7_7bDwW9aNQ@mail.gmail.com>
References: <git-pr-165-cordova-cli@git.apache.org>
	<20140417214643.E0C2B9923CF@tyr.zones.apache.org>
	<CAKP6k2vfWUSoQngM5LrhzS1udT36VcaZnofL8+w6245DhmOWWA@mail.gmail.com>
	<CAEeF2Tc=mc5cPGJ_bTYzhuUW7RvWLEQOE2hTH_p7_7bDwW9aNQ@mail.gmail.com>
Date: Fri, 18 Apr 2014 13:11:28 -0700
Message-ID: 
 <CA+X37cmLxTninngSUM7CLtfHYHpTqhteUaC4v7QL+03=8ib65w@mail.gmail.com>
Subject: Re: [GitHub] cordova-cli pull request: CB-6469 - Restore plugins from
 config.xm...
From: Gorkem Ercan <gorkem.ercan@gmail.com>
To: "dev@cordova.apache.org" <dev@cordova.apache.org>
Content-Type: multipart/alternative; boundary=047d7b5d83913f403004f756c13a
X-Virus-Checked: Checked by ClamAV on apache.org

--047d7b5d83913f403004f756c13a
Content-Type: text/plain; charset=UTF-8

On Thu, Apr 17, 2014 at 3:46 PM, Michal Mocny <mmocny@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.


> - 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.



> I haven't put the PR through testing yet.
>
>
> On Thu, Apr 17, 2014 at 5:54 PM, Jesse <purplecabbage@gmail.com> wrote:
>
> > Yeah, that looks weird.
> >
> > @purplecabbage
> > risingj.com
> >
> >
> > On Thu, Apr 17, 2014 at 2:46 PM, kamrik <git@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 infrastructure@apache.org or file a JIRA
> > ticket
> > > with INFRA.
> > > ---
> > >
> >
>

--047d7b5d83913f403004f756c13a--