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

Support grouping arguments to higher-order tasks #1138

Closed
mtopolnik opened this issue Apr 9, 2013 · 13 comments
Closed

Support grouping arguments to higher-order tasks #1138

mtopolnik opened this issue Apr 9, 2013 · 13 comments

Comments

@mtopolnik
Copy link
Collaborator

If I write the following;

$ lein do clean, thrush uberjar, upload

do will parse it into

["clean"] ["thrush" "uberjar"] ["upload"]

which will prevent me from combining do with thrush. It would be more useful if do realized that thrush is a higher-order task and turned everything over to it for further parsing:

["clean"] ["thrush" "uberjar," "upload"]

I have already implemented this in my lein-nix project and it works very well, for example, with this complex expression:

$ lein xdo git-check-clean, \
  thrush version-update :release, edit-version, \
  xdo deploy clojars, commit "New release", tag, \
  thrush version-update :new-snapshot, edit-version, \
  xdo commit "New snapshot", push

where xdo is my do with the changed semantics. The above is a complete deployment script laid out as a single lein invocation, which can be easily turned into an alias.

Please consider applying the same semantics to leiningen's do.

@hypirion
Copy link
Collaborator

hypirion commented Apr 9, 2013

Ahh, dangling commas. Just looking at :higher-order won't do, as both update-in and with-profile doesn't use commas, and you'd most likely not want to keep the comma for them. Perhaps adding in ^{:delimiter \,} to denote tasks using comma for delimiting values?

How should the scope be defined though? Should aliases be kept lexical, or should they be dynamic?

If implemented, this should also be provided from or done within leiningen-core, as every task with a :delimiter would need a function to handle these things for subtasks.

Then again, using aliases ({:aliases {"uberupload" ["thrush" "uberjar," "upload"]}}) would solve this problem without adding more complexity to the system as a whole.

@mtopolnik
Copy link
Collaborator Author

Nesting aliases is a good concept, I agree. I didn't get your question about the scope of aliases.

As for HOTs that do not chain, this is something I did take into account:

lein thrush uberjar, upload, with-profile :dev do compile, test

This will parse correctly into

(thrush (uberjar) (upload)) 
(with-profile :dev (do (compile) (test))

I don't see this proposal as more complexity; the complexity is exactly the same, just more useful.

@hypirion
Copy link
Collaborator

hypirion commented Apr 9, 2013

So, what I wonder about is how e.g. lein do with-profile +dev test, deploy clojars would expand. My guess is that people would like

(do (with-profile "+dev" "test")
    (deploy "clojars"))

But without proper handling, this may turn into something more messy.

When I talked about dynamic vs lexical scoping, I was thinking of something like this:

;; part of project
{:aliases {"alias" ["foo" "bar," "baz"] ...} ...}

;; Running `lein do alias, test`
;; Lexical scoping:
(do (foo "bar," "baz")
    (test))

;; Dynamic scoping:
(do (foo "bar") (baz) (test))

@mtopolnik
Copy link
Collaborator Author

lein do with-profile +dev test, deploy clojars

This is a fair point; the current semantics are better suited to the nesting of non-chaining HOTs and my proposal would shift that in favor of the chaining of chaining HOTs. I did experiment with additional meta for chaining tasks, but in the end thought it wouldn't be needed. This counterexample makes me doubt that conclusion.

;; Running `lein do alias, test`
;; Lexical scoping:
(do (foo "bar," "baz")
    (test))
;; Dynamic scoping:
(do (foo "bar") (baz) (test))

This is about the order of application of alias expansion vs. argument grouping. My idea was to always start from what the user entered and let alias expansion happen late, within each HOT. So that would correspond to your "lexical scoping". However, there may be related complications when the alias resolves to a HOT and thus takes over the parsing of the rest of the command line: it may result in unintuitive behavior.

@mtopolnik
Copy link
Collaborator Author

I'm closing this for now because more investigation is needed to formulate a better proposal.

@hypirion
Copy link
Collaborator

@mtopolnik - I'll reopen it since it would most likely be forgotten if we don't have an open issue on it.

I also have some ideas on how to handle this without getting it too dirty.

@hypirion hypirion reopened this Apr 10, 2013
@technomancy
Copy link
Owner

I think this should be better supported, but there are a few questions we should address first.

  • Should ^:higher-order be used as more than just documentation? Right now it's just there to signal to a human reader.
  • Should we assume higher-order tasks use commas as separators? The update-in task uses double dashes.
  • Would a solution around escaping be simpler? $ lein do clean, thrush uberjar\, upload perhaps? I feel this might be clearer at the expense of being less pretty.

@mtopolnik
Copy link
Collaborator Author

update-in uses double-dash as quite a different type of delimiter, and one which is in line with the general *nix convention for it. The escaped comma seems like an OK approach; another idea would be to use an explicit end of block marker because that would give arbitrary nesting levels.

@mtopolnik
Copy link
Collaborator Author

After some thought, some meta will have to be used to distinguish tasks that employ the comma delimiter (not
:higher-order, but something specifically signalling the usage of the comma delimiter).

If this is not used then other mechanisms would need to be introduced, and it would be only getting more complex.

As far as signalling the end of a task chain, consider this:

lein do clean, thrush uberjar, upload , tag

Notice the space before the last comma.

@mtopolnik
Copy link
Collaborator Author

More on the "escape the comma" approach:

  1. we shouldn't choose the backslash since it is already taken as the escaping char at the command line;
  2. how exactly should things happen there? Will do unescape before invoking a task, or will the invoked task unescape as the first thing it does?
  3. How should nesting be handled? Double-escaping, like \\,?
  4. I feel some problems lurking behind this approach... like programmatically building a lein invocation from fragments, getting lost in the number of escapes needed for each fragment.

@technomancy
Copy link
Owner

Re-reading this discussion, it sounds just like an argument precedence issue.

If we're going to support arbitrary nesting, it should probably be specified using matching delimiters.

@mtopolnik
Copy link
Collaborator Author

This whole issue would be a triviality if we didn't face the constraints of the command line. All the braces are taken and would need escaping. Maybe just live with escapes?

@technomancy
Copy link
Owner

Looking back at this, I don't think this complexity is justified given that this ambiguity does not exist in aliases, which are a better way to accomplish this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants