Apply --depth and --compact flags to --tasks-json and --tasks #75

sttk · 2016-04-05T15:44:33Z

This is a PR about the issue #57

sttk · 2016-04-06T14:21:05Z

npm install EPERM errors frequently occured on appveyor.
It seems that this error can be fixed by using npm3.
So I modified appveyor.yml to install latest npm.

npm/npm#9696

sttk · 2016-04-07T13:40:30Z

Sorry. I forgot to rename the second argument of logTasks.

sttk · 2016-07-11T20:23:18Z

I've reflected the change of PR #87 for undertaker v1.0.0 and added --no-sort flag.

phated · 2016-07-15T17:49:00Z

This will need to be rebased on top of the merged undertaker 1.0 fixes.

sttk · 2016-07-16T08:38:40Z

I've rebased this on the current master branch.

phated · 2017-04-22T23:57:20Z

@sttk This will probably need many changes to support only through the config files.

sttk · 2017-04-23T00:59:22Z

@phated O.K. I'm going to address this from now.

sttk · 2017-04-23T12:39:18Z

@phated I've finished reflecting the latest master branch.

phated · 2017-04-23T23:32:41Z

@sttk thanks for rebasing. However, I don't think we should expose the --compact, --depth or --no-sort options as command line flags. Instead, could you implement this to only be configurable from .gulp.* files?

phated · 2017-04-23T23:36:46Z

Sorry, I forgot that we shipped --depth but it looks like --compact and --no-sort are being added. I'm not sure I want to keep expanding the flags but maybe those are a must-have for @segrey's use case?

sttk · 2017-04-24T20:11:30Z

In his use case, gulp would be called from a program. So I think --compact flag is needed rather than config.

About --depth flag, I suppose users want to use both flag and config. For example, they usually specify depth=3 with config, and sometimes show full depth tree with the flag.

About --no-sort flag, I think it may be good to be config only.

sttk · 2017-04-24T20:28:04Z

@phated Could you define config keys of compact, depth, sort/no-sort?

phated · 2017-04-25T16:02:04Z

@sttk if you are going to support --compact and --depth as flags, they should just be flags.compact and flags.depth; however, do we want to make them more descriptive (e.g. --compact-tasks and --tasks-depth)? Do we really need the sort/no-sort option? I'm not sure who would want to turn sorting off.

sttk · 2017-04-26T14:39:28Z

I also think --compact-tasks and --tasks-depth are better. These names make more clear what the flags is for.

no-sort was requested by @erikkemperman in the issue #50.

erikkemperman · 2017-04-26T15:16:05Z

Yeah, and actually I might even argue the default should be to respect the order of tasks in my gulpfile, where I tend to try and find some sensible structure and keep related tasks near one another. Then I'd like to see the same order when listing the tasks. But maybe that's just me?

phated · 2017-04-26T17:09:48Z

@erikkemperman I think that's a sane default, which I believe is equivalent to "no sort" because I just iterate over the object (it's not guaranteed to be in insert order, but highly likely).

erikkemperman · 2017-04-26T18:12:05Z

Maybe I misunderstand, I thought the proposal here is to do alphabetical sorting by default, and have a "no-sort" option to get file-order. I would prefer the other way around. But again, maybe that's just me and anyway it's not a big deal :-)

phated · 2017-04-26T18:14:19Z

@erikkemperman the original implementation sorted by default. This PR is adding the option to "disable" that but I'm suggesting we invert that assumption (remove sorting by default and add a --sort-tasks flag).

erikkemperman · 2017-04-26T18:18:34Z

Excellent, that's what I would expect :-)

phated · 2017-05-12T18:38:20Z

@sttk are you still planning to update this PR based on the discussion?

sttk · 2017-05-12T20:44:32Z

@phated Sorry that I'm late to update.

I implemented and I'm testing about this PR from now.

I'm changing the flag names to: --compact-tasks, --tasks-depth, --sort-tasks, and adding the configs: flags.compactTasks, flags.tasksDepth, flags.sortTasks in accordance with the flag names.

Please let me know if there are flags or configs that should be renamed or removed,

phated · 2017-05-12T20:53:46Z

@sttk I think all of those flags & configs are good.

sttk · 2017-05-13T10:05:14Z

@phated I've rebased and updated.

tkellen · 2017-05-13T12:45:07Z

docs/CLI.md

    Print the task dependency tree, in JSON format, for the loaded gulpfile. The [path] argument is optional, and if given writes the JSON to the path.

+**--depth** [number]
+    Specify the depth of the task dependency tree to print. This flag can be used with --tasks or --tasks-json.


This should be tasks-depth, right?

tkellen · 2017-05-13T12:45:39Z

docs/CLI.md

+**--depth** [number]
+    Specify the depth of the task dependency tree to print. This flag can be used with --tasks or --tasks-json.
+
+**--compact**


This should be compact-tasks, right?

sttk · 2017-05-13T13:25:16Z

@tkellen Thanks! I've modified what you pointed out.

phated

Few changes needed.

phated · 2017-06-02T22:02:29Z

lib/shared/cliOptions.js

    desc: chalk.gray(
      'Print the task dependency tree for the loaded gulpfile.'),
  },
-  depth: {


This is a breaking change. Can we leave it in as an alias? Does yargs support aliases in the definition?

If we alias this, it'll need to be documented and noted as deprecated.

I rethought about flag names that --depth, --compact and --sort are better than --tasks-depth, --compact-tasks and --sort-tasks. When we use these flags, we type a command like gulp --tasks --tasks-depth. The word "tasks" is redundant for users, I think.

I think good about the config names, but it is possible to group these flags like flags.tasks.depth, flags.tasks.sort, flags.tasks.compact (in .gulp.json, { flags: { tasks: { depth: 3, sort: true, compact: true } } }).

What do you think?

I'm fine with being verbose on the flag names but I think we just need to alias depth because we've previously defined it and removing it would be a breaking change (and I'd have to publish as 2.0.0)

I got it. I'll add the alias depth.

phated · 2017-06-02T22:13:07Z

lib/versioned/^3.7.0/index.js

        return gulpInst.tasks[task].fn;
      });
    }
+    if (opts.tasksJson) {


Can this addition be moved to a separate commit/PR? I'm glad we are adding it but I want it to show up in the changelog properly.

I got it. I'll remove this part from this PR.

phated · 2017-06-02T22:23:27Z

test/expected/flags-tasks-depth4.txt

+└─┬ default
+  └─┬ <parallel>
+    ├─┬ taskC
+    │ └── <series>


Is this considered part of the depth?

Yes. depth flag cuts deeper tasks forcely.

…ort.

sttk · 2017-06-10T07:04:48Z

I've rebased and modified.

phated · 2017-06-12T18:52:45Z

Looks good, great work again @sttk

I'm going to squash this, feel free to submit the --tasks-json feature for V3 afterwards.

sttk · 2017-06-12T19:27:57Z

@phated Thank you for merging.

phated · 2017-06-12T19:44:09Z

@sttk No problem. I'm noticing some timeout problems (for a couple tests) on Appveyor. Did you run into that at all?

sttk · 2017-06-13T14:43:39Z

@phated I sometimes encounter timeout problems on Appveyor, too. And in that case, I try test 2 or 3 times. I don't know this workaround.

…ort & add sort flag (closes #57) (#75)

sttk changed the title ~~Add --depth and --compact flags to --tasks-json and --tasks~~ Apply --depth and --compact flags to --tasks-json and --tasks Apr 7, 2016

erikkemperman mentioned this pull request Jun 11, 2016

Remove shebang and x mode from index.js (fix #80); fix tests #81

Closed

sttk force-pushed the modify_for_tasks_json_norcr branch from 91e2de4 to 14d8855 Compare July 11, 2016 15:33

sttk force-pushed the modify_for_tasks_json_norcr branch from 14d8855 to 0e01cc9 Compare July 16, 2016 08:30

sttk force-pushed the modify_for_tasks_json_norcr branch from 0e01cc9 to e2038b3 Compare April 23, 2017 12:21

sttk force-pushed the modify_for_tasks_json_norcr branch from e2038b3 to 72c7757 Compare May 13, 2017 09:39

tkellen reviewed May 13, 2017

View reviewed changes

erikkemperman mentioned this pull request May 30, 2017

Build: make sure to test all callback args #114

Closed

phated requested changes Jun 2, 2017

View reviewed changes

sttk and others added 7 commits June 9, 2017 23:21

Modify --tasks and --tasks-json outputs to be able to compact and uns…

f71f9b9

…ort.

Rename flags and add configs

4bf2379

Modify mistakes in docs/CLI.md

bb1b815

Add alias --depth

6342234

Not add --tasks-json for v3 yet

f9c1d36

Ensure all callback arguments are tested for added test cases

455c958

Modify documents

305b53d

sttk force-pushed the modify_for_tasks_json_norcr branch from f311150 to 305b53d Compare June 10, 2017 06:44

phated approved these changes Jun 12, 2017

View reviewed changes

phated merged commit 86f5df8 into gulpjs:master Jun 12, 2017

sttk deleted the modify_for_tasks_json_norcr branch June 12, 2017 19:28

phated pushed a commit that referenced this pull request Dec 21, 2017

Update: Apply depth/compact flags to tasks-json/tasks w/ .gulp.* supp…

a096813

…ort & add sort flag (closes #57) (#75)

phated pushed a commit that referenced this pull request Dec 21, 2017

Update: Apply depth/compact flags to tasks-json/tasks w/ .gulp.* supp…

0a40176

…ort & add sort flag (closes #57) (#75)

phated pushed a commit that referenced this pull request Dec 21, 2017

Update: Apply depth/compact flags to tasks-json/tasks w/ .gulp.* supp…

2f8e933

…ort & add sort flag (closes #57) (#75)

phated pushed a commit that referenced this pull request Dec 21, 2017

Update: Apply depth/compact flags to tasks-json/tasks w/ .gulp.* supp…

d5ced18

…ort & add sort flag (closes #57) (#75)

phated pushed a commit that referenced this pull request Dec 21, 2017

Update: Apply depth/compact flags to tasks-json/tasks w/ .gulp.* supp…

4c4abfd

…ort & add sort flag (closes #57) (#75)

Uh oh!

Apply --depth and --compact flags to --tasks-json and --tasks #75

Apply --depth and --compact flags to --tasks-json and --tasks #75

Uh oh!

Conversation

sttk commented Apr 5, 2016

Uh oh!

sttk commented Apr 6, 2016

Uh oh!

sttk commented Apr 7, 2016

Uh oh!

sttk commented Jul 11, 2016

Uh oh!

phated commented Jul 15, 2016

Uh oh!

sttk commented Jul 16, 2016

Uh oh!

phated commented Apr 22, 2017

Uh oh!

sttk commented Apr 23, 2017

Uh oh!

sttk commented Apr 23, 2017

Uh oh!

phated commented Apr 23, 2017

Uh oh!

phated commented Apr 23, 2017

Uh oh!

sttk commented Apr 24, 2017

Uh oh!

sttk commented Apr 24, 2017

Uh oh!

phated commented Apr 25, 2017

Uh oh!

sttk commented Apr 26, 2017

Uh oh!

erikkemperman commented Apr 26, 2017

Uh oh!

phated commented Apr 26, 2017

Uh oh!

erikkemperman commented Apr 26, 2017

Uh oh!

phated commented Apr 26, 2017

Uh oh!

erikkemperman commented Apr 26, 2017

Uh oh!

phated commented May 12, 2017

Uh oh!

sttk commented May 12, 2017

Uh oh!

phated commented May 12, 2017

Uh oh!

sttk commented May 13, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sttk commented May 13, 2017

Uh oh!

phated left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!