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

Allow if on tasks #225

Open
davesteinberg opened this issue Jun 20, 2020 · 6 comments · May be fixed by #229
Open

Allow if on tasks #225

davesteinberg opened this issue Jun 20, 2020 · 6 comments · May be fixed by #229

Comments

@davesteinberg
Copy link

davesteinberg commented Jun 20, 2020

I have an enhancement to propose: Allowing an if property (which would work just like the link option) on the top-level tasks, alongside each directive. If the executed command is not successful, the whole task would be skipped.

The idea is that you could define sets of links, or shell commands, or whatever task, for an environment/profile/whatever. My use case is that I'd like to have a task with links that are always created, and then another set just for personal machines, and a third just for work machines. I have a different .gitconfig file between the two, plus a .pairs file for work, plus more...Since I can't put two different entries for the same file under a single link task, I need to have different task for these cases, anyway. So, it would be wonderful if I didn't have to repeat the same if option again and again and again.

For exmaple:

- link:
    # all my common links

- if: [[ ! $PROFILE ]]
  link:
    ~/.gitconfig:

- if: [[ $PROFILE = work ]]
  link:
    ~/.gitconfig: gitconfig.work
    ~/.pairs
    # other work-only links

Plus, I could imagine this being useful with other directives -- definitely for shell and create, and probably for many plugins, too.

I think this small enhancement would make dotbot so much more flexible and powerful.

@davesteinberg davesteinberg changed the title Allow if on a directive Allow if on tasks Jun 20, 2020
@anishathalye
Copy link
Owner

That's an interesting suggestion -- I can certainly see how it could be useful to expand the if beyond link. I think this could be implemented in theory -- Dotbot could look at groups of commands within an array, and if there's one that's an if, test the property and only execute the others if the evaluation returns true. That would be straightforward enough. But I think that would complicate the semantics quite a bit.

Dotbot has a simple declarative-ish YAML config file, and if we add something like this, we'll be slowly moving towards a programming language embedded in YAML (this has happened with Ansible, for example), and I think this eventually becomes unmanageable. YAML is a pretty good config file format, but it's not a great programming language -- it's not really designed for that.

If we're moving towards a more featureful DSL with general-purpose programming language constructs like structured control flow etc., another option might be to have a embedded DSL in a real programming language. Python is a decent DSL host; we could even imagine migrating to a language explicitly designed for designing languages, like Racket (though I don't think we'll do this in practice for practical purposes, because everyone has Python preinstalled but nobody will have Racket).

There's a bit more discussion on this topic in #35 (comment), with some examples of what this syntax could look like. I could imagine a future version of Dotbot could support both, a simple YAML config file for simple situations, and the full power of the Python programming language for more complicated situations.

I'd love to hear your thoughts on this.

And thanks for opening this issue -- it's gotten me thinking again about adding these kinds of more powerful features to Dotbot.

@kirtangajjar
Copy link

@anishathalye How about only limiting it to if and not allowing any other constructs? I think it's the most common use case and will satisfy most use cases. Have you encountered any other common problem which requires other constructs?

@davesteinberg
Copy link
Author

@anishathalye Thanks for considering my proposal and sharing your reaction. I understand your hesitation -- I can see how my example looks a bit more like a programming language -- but I see it differently. I don't think my proposal would make Dotbot's YAML format any less declarative, and I hope I can convince you of this.

As I see it, link objects can already have an if property, however imperative or declarative that is. Recognizing that such a property could equally apply to a whole containing task object, and supporting that by hosting the property up two levels, actually makes the property feel more declarative to me. It seems analogous to the way that I can hoist an HTML style up from a particular div to a containing div, or even to the body or html, if it should apply more broadly.

Ultimately, it allows me to achieve the same thing that I could today, only with much less repetition. And, as a side effect, it also allows the if property to apply to other directives, not just link, where it could be just as useful.

I really see it as a tiny enhancement to the configuration format that really doesn't change the sorts of things you can describe at all. You certainly won't see me arguing for control flow constructs like loops. I agree that would be a bad idea, but it doesn't feel like a slippery slope to me -- I can't see how it would fit at all with how Dotbot works.

Thanks for pointing to the Dotbot 2.0 issue. There are a lot of interesting ideas there. I'm with the folks who preferred to stick with the current YAML format. I'm a big fan: It's simple and it's very accessible to pretty much any developer, regardless of what language(s) they work in. I think that's very valuable. Personally, I'm not a Python programmer, so switching to a Python DSL would be a barrier for me.

Even though I'm not a Python programmer, I thought it would be worthwhile to see what implementing my proposal might look like, so I took a stab at it. It was such a tiny change that even I was able to do it. I'm going to open a PR for you to have a look at. Hopefully having something concrete to consider will be helpful.

Thanks again!

@davesteinberg davesteinberg linked a pull request Jul 13, 2020 that will close this issue
@davesteinberg
Copy link
Author

davesteinberg commented Jul 13, 2020

After I finished my PR and comment last night, I took a look at issue #81 based on the mention above, and that expanded my thinking a bit more and maybe helped me to see some of the disconnects.

I do think what folks are asking for there is closely related to what I'm asking for here. To put a finer point on what I was trying to say in my previous comment: In both cases, I think it's not control flow, but rather more powerful composition. It's the ability to group some related actions and identity and describe them as a unit. And as I see it, composition is very much a declarative concept.

The main difference between what's been going there vs. what I'm proposing here is that I see the existing concept of a task as that unit of composition. It already can, itself, compose multiple actions. All it's missing to be super useful is a name and a way of declaring in which situations it applies (and my suggestion was to reuse the existing if mechanism for that).

What they're talking about over there is a unit of composition that's bigger than tasks. But what's not clear to me is if they're thinking about it that way because they don't know that tasks can already compose multiple actions, or if they need something more than that. In fact, it's not even clear to me if tasks are intended to compose multiple actions. The only reason I know they can is from looking at the code (dispatcher.py:24). There aren't any tasks with multiple actions shown in the docs.

So, I wonder if that might be the source of discomfort and disconnect with these sorts of suggestions -- the question of what tasks are actually meant to be and whether a mechanism for composition (rather than just a list of actions) is desirable..

As I see it, yes, a mechanism for composition would be highly beneficial in a lot of cases, and tasks are very well suited to be that mechanism. It wouldn't complicate the model at all for users who don't need them, and the changes to the code would be minor.

With all this in mind, I've come up with minor revision to my idea that's a little more explicit and unambiguous:

- task: common
  actions:
    link:
      # all my common links

- task: personal profile
  if: [[ ! $PROFILE ]]
  actions:
    link:
      ~/.gitconfig:
    shell:
      # run something personal

- task: work profile 
  if: [[ $PROFILE = work ]]
  actions:
    link:
      ~/.gitconfig: gitconfig.work
      ~/.pairs
      # other work-only links

I'm adding a task property that allows you to identify the task. When that's present, you can also use an if property to declare the task's applicability, and the task's actions move under an explicit actions property. This way, there's no ambiguity and you don't have to worry about task properties stepping on action names from plugins.

Of course, all of this is optional. I could have declared the common task without a name just like before:

- link:
    # all my common links

Users who just want a flat list of actions and no composition can continue using this format.

What would you think of this approach? If you think it might be worth pursuing, I'd be happy to update my PR to implement it.

@davesteinberg
Copy link
Author

I've updated #229 to implement my new proposal. I think it's still a really simple, clean change.

@anishathalye
Copy link
Owner

I'm with the folks who preferred to stick with the current YAML format. I'm a big fan: It's simple and it's very accessible to pretty much any developer, regardless of what language(s) they work in. I think that's very valuable. Personally, I'm not a Python programmer, so switching to a Python DSL would be a barrier for me.

Yeah, this is a good point. I think Python is among the more accessible languages (even though I love Racket, I don't think I'd release a Dotbot 2.0 implemented in it), but yeah, YAML is probably better understood.

but rather more powerful composition

This is the thing that "true programming languages" handle pretty well, but data serialization languages don't do quite as well. Have a generic if construct, and it applies anywhere (can have an if at the top level, can nest it deep inside somewhere, ...).

In fact, it's not even clear to me if tasks are intended to compose multiple actions. The only reason I know they can is from looking at the code (dispatcher.py:24). There aren't any tasks with multiple actions shown in the docs.

Heh, that's a good question. Yes, in practice, each task ends up having exactly one action. This is a bit of an artificial distinction that arose from the need to have actions be named (so we know whether to link or shell or whatever) but also the desire to have actions be executed in the order written, and also to allow multiple actions with the same name. This is another weird thing that's a result of using a pretty rigid data serialization format (YAML/JSON) as oppoosed to something more customized for the use case at hand. What I wanted was a (ordered) list of pairs (task name, arguments), but YAML dictionaries are unordered, so we couldn't use just a dictionary at the top level. So it looks like everything is "indented" one level in, where we have a list of dictionoaries where each dictionary has a single key/value pair. If someone didn't care about ordering of task execution, and also didn't list any key twice (e.g. no multiple links), then it would be fine if a task had multiple actions.

With all this in mind, I've come up with minor revision to my idea that's a little more explicit and unambiguous: [...]

That looks pretty clean, though perhaps a bit verbose for my tastes (I think I wouldn't personally need that kind of complexity, so I'd have a single task). Again, a challenge with the YAML format is that actions: is unordered, and any given action can only appear once.

In the strictest sense, this is not backwards compatible, because technically "task" and "if" are legal plugin names. But nobody is using those silly plugin names in practice, so I don't think it's a problem.

(more specific comments in #229)

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

Successfully merging a pull request may close this issue.

3 participants