Working with features #194

alanpoon · 2018-01-25T03:46:38Z

ordian

Thank you for contributing to cargo-edit, @alanpoon!

Comments:

I've left some inline comments.
We had a discussion in Support for working with features #193 about the format of output. Ideally, we want to extract the information about available features from the crate's Cargo.toml and print a nicely formatted feature list, but that can be done in a separate PR.

ordian · 2018-01-25T17:12:08Z

README.md

@@ -71,7 +71,8 @@ Specify what crate to add:
                            `cargo add [email protected]`.
    --git <uri>             Specify a git repository to download the crate from.
    --path <uri>            Specify the path the crate should be loaded from.
-


I think we should follow cargo in describing the feature flag:

$ cargo build --help 2>&1 | grep -e "--features" --features FEATURES Space-separated list of features to also build

(and maybe replace also build with add)

ordian · 2018-01-25T17:12:25Z

.travis.yml

@@ -44,4 +44,4 @@ env:
  - secure: XXYE8x015gOSJ+DD9FeCdEKQTeU6mZ5j47Nh1ncZTkdN6+RnrN/DxAwccvh9u6t8/V6WN868AHIvTc0nA031mqwnh6vcQDr3Ak8ydJCTM8KFgwsuESokSZYkbiiQd/FWTBri/P9H5jejCw3aSFQZUVeCpu2Zxp0N9/Jn3ky7NVknBOVdz21jRC/KivBkpbvOCj8iEtGZuYSQu3Gpq1wbuZaqDdgg6JVvrYy7LbUNGe0rHMjCcyuiutqp6R8zIQoC7p2kmoHkXrTaDKQrpmSi+uLJJE7KqML9tFFnvjvmgroRYDCk0gc/fr/GunikvUkcczBBSsVXWj66EpriQxwdEnI1uhNtKUOGALIAMiLz2k0rbLl1wFLOvFlhEPhDGvt+mw+h2BIxoyy9mwVioYym7V8pksw0UuCuLuyAp1Rc89iT6/N3HfMFPZeTBeudktUb5ijHgJyobVwakxKXgKqELbrhUhmzBf8fwtmPH+Lnganyrz4qcKBnuUl2fSp6MIaGIeSZZwigggV8QSXw9u/1c32RhQs47ZDRJX2lzEzVyAVZ0liyn32hWFI6Ctur4jNV7LWtOcDqdYZZfqyZOL/HehNFtDoJfrYsp/KZvmBipmOkrNsKCKEAdvUoeTDXo4ME31tjGtiPbHIdRuBJJqt38gm1AKyoVJKp2KFGDsI0djA=
 branches:
  except:
-  - staging.tmp


Could you revert this change?

ordian · 2018-01-25T17:16:06Z

src/bin/add/args.rs

        }


        let dependency = if !crate_name_is_url_or_path(&self.arg_crate) {
-            let dependency = Dependency::new(&self.arg_crate);
+                let dependency = Dependency::new(&self.arg_crate);


This formatting seems odd (why 8 whitespaces?).

ordian · 2018-01-25T17:26:16Z

src/dependency.rs

@@ -14,6 +14,7 @@ pub struct Dependency {
    pub name: String,
    optional: bool,
    source: DependencySource,
+    features: Vec<String>,


We need to distinguish an empty feature list from the default feature list (not specifying the feature flag).

Hi Ordian,
Thanks for your suggestion. "cargo list" or "cargo read-manifest" can only read the local cargo.toml manifest file. When you add a dependency to your project, you don't save a copy of the manifest file of the dependencies. Without the manifest file of the dependency, there is no way of returning the list of the available features. I think this enhancement is only useful for experienced users who would check out the available features themselves. I am only motivated in using cargo-edit because Cargo does not support Target specific features. rust-lang/cargo#1197
With cargo-edit being able to specify features, I can use continuous integration to build various target builds with dependencies with various features.

We download Cargo.toml locally for git dependencies in cargo-edit (and download json for crates.io dependencies), so a proper way to extract feature list would be to create a function get_features_from_manifest similar to get_name_from_manifest. But that can be done in a separate PR.

The point of the comment is different: if user of cargo-edit does not specify --features when adding a dependency, he probably wants a default feature list, not the empty one. Btw, current CI failure is relevant.

ordian · 2018-01-25T17:32:05Z

src/dependency.rs

@@ -95,7 +105,13 @@ impl Dependency {
                if self.optional {
                    data.get_or_insert("optional", optional);
                }
-
+                if !self.features.is_empty() {
+                    let mut to_array = toml_edit::Array::default();


toml_edit::Value implements FromIterator, so we can simplify this:

use std::iter::FromIterator; let features = toml_edit::Value::from_iter(self.features.iter().cloned()); data.get_or_insert("features", features);

but we need to specify feature list only if user specified it.

ordian · 2018-01-25T17:34:04Z

tests/cargo-add.rs

+    // dependency present afterwards
+    let toml = get_toml(&manifest);
+    let val = &toml["dependencies"]["cargo-edit"]["features"];
+    let result = if let toml_edit::Item::Value(toml_edit::Value::Array(ref arr_z)) = *val {


I haven't tried this, but it should work:

let val = toml["dependencies"]["cargo-edit"]["features"][0].as_str(); assert_eq!(val, Some("jui"));

ordian · 2018-01-31T11:05:20Z

Thanks for the changes. Do you think we should also add a no-default-features flag for consistency?

cc @killercup

qmx · 2018-01-31T23:32:38Z

README.md

@@ -71,7 +71,8 @@ Specify what crate to add:
                            `cargo add [email protected]`.
    --git <uri>             Specify a git repository to download the crate from.
    --path <uri>            Specify the path the crate should be loaded from.
-
+    --features <FEATURES>   Space-separated  list of features to add
+


stray whitespace here

qmx · 2018-01-31T23:42:12Z

this works as expected, we might want to change the user output though

❯ cargo add cursive --features="termion-backend"   
      Adding cursive v0.7.5 to features termion-backend dependencies

ordian · 2018-02-01T00:55:35Z

Thanks @qmx, good catch! But we may change the output to the form mentioned in #193 in the future PR.

@alanpoon, could you add an output test similar to this one, fix the ordering of features in the output and add a flag no-default-features? Also could you please revert all unrelated changes, like formatting and travis?

ordian

Could you also please rebase and squash this PR down to a single commit when you're ready? Thanks!

ordian · 2018-02-07T12:54:45Z

src/bin/add/main.rs

@@ -97,6 +98,9 @@ fn print_msg(dep: &Dependency, section: &[String], optional: bool) -> Result<()>
    if optional {
        write!(output, " optional")?;
    }
+    if let Some(f) = features {
+        write!(output, " features {}", f)?


Could you place this output after line 109 (and change writeln to write), so that the output would be

... to dependencies with features: jui

rather than

to features jui dependencies

richard-uk1 · 2019-07-10T09:41:53Z

Friendly ping!

This would be a nice feature to have. @alanpoon if you like I could adopt the PR?

alanpoon · 2019-07-10T14:39:54Z

Hi derekdreery,
Sure, do I need to do anything?

polarathene · 2019-12-01T01:25:49Z

I'd also like to see this PR get merged, it's been a while since it was submitted and thus getting stale with conflicts. The changes that were requested were done? Can it be reviewed again and if valid then re-based and merged?

ordian · 2019-12-01T11:45:45Z

Master has changed substantially, this PR needs rebasing and then I'll review it.

@alanpoon

Credit to @alanpoon for the original implementation of this feature. This patch is a rebase of the original PR (killercup#194) with minor tidying up. Also contains some unrelated clippy and fmt fixes for rust 1.41.0.

@alanpoon

* add --features='nose mouth' Credit to @alanpoon for the original implementation of this feature. This patch is a rebase of the original PR (#194) with minor tidying up. Also contains some unrelated clippy and fmt fixes for rust 1.41.0. * mention cargo-feature; update README * forbid specifying multiple crates with --features * fixup! forbid specifying multiple crates with --features

ordian · 2021-09-13T09:55:44Z

I believe this was superceded by #395

alanpoon and others added 7 commits January 24, 2018 00:16

added features flage

411ffa9

fixed test for features

b1c7e3b

correct for nightly

b3b416c

format with rustfmt-nightly

45ffd72

change rustfmt in travis

0fc815c

added force to install rustfmt

311fc15

revert travis

24966a8

ordian reviewed Jan 25, 2018

View reviewed changes

alanpoon and others added 6 commits January 28, 2018 15:33

revert travis, simplify code

164de17

Merge branch 'master' of https://github.com/alanpoon/cargo-edit

f30a63a

added FromIterator

56e6907

misspelled jui

67349ad

add only if self.features is empty

e85b85a

fix code diff

1895d1e

qmx reviewed Jan 31, 2018

View reviewed changes

alanpoon added 5 commits February 1, 2018 09:13

remove straywhitespace

547d93b

add_prints_message_for_features_deps

68f0a64

resolve format and add newline at the files

50aa8aa

refix newline at the end

cf2444c

touchup format

5bb92f3

ordian reviewed Feb 7, 2018

View reviewed changes

alanpoon added 4 commits February 12, 2018 21:53

change feature output

4ba1f6d

fix output

17adeb8

Working with features

21676e8

Merge branch 'master' of https://github.com/alanpoon/cargo-edit

c7f287c

bjgill mentioned this pull request Feb 16, 2018

Upgrade Rustfmt + clippy #202

Merged

alsuren mentioned this pull request Mar 26, 2020

"Working with features" updated #395

Merged

5 tasks

ordian closed this Sep 13, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Working with features #194

Working with features #194

alanpoon commented Jan 25, 2018

ordian left a comment •

edited

Loading

ordian Jan 25, 2018

ordian Jan 25, 2018

ordian Jan 25, 2018

ordian Jan 25, 2018

alanpoon Jan 28, 2018

ordian Jan 28, 2018

ordian Jan 25, 2018

ordian Jan 25, 2018 •

edited

Loading

ordian commented Jan 31, 2018

qmx Jan 31, 2018

qmx commented Jan 31, 2018

ordian commented Feb 1, 2018 •

edited

Loading

ordian left a comment

ordian Feb 7, 2018

richard-uk1 commented Jul 10, 2019

alanpoon commented Jul 10, 2019

polarathene commented Dec 1, 2019

ordian commented Dec 1, 2019

ordian commented Sep 13, 2021

Working with features #194

Working with features #194

Conversation

alanpoon commented Jan 25, 2018

ordian left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ordian Jan 25, 2018 • edited Loading

Choose a reason for hiding this comment

ordian commented Jan 31, 2018

Choose a reason for hiding this comment

qmx commented Jan 31, 2018

ordian commented Feb 1, 2018 • edited Loading

ordian left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

richard-uk1 commented Jul 10, 2019

alanpoon commented Jul 10, 2019

polarathene commented Dec 1, 2019

ordian commented Dec 1, 2019

ordian commented Sep 13, 2021

ordian left a comment •

edited

Loading

ordian Jan 25, 2018 •

edited

Loading

ordian commented Feb 1, 2018 •

edited

Loading