Re: [Fab-user] Patch for prompt and require

[Niklas Lindström]

Hi again!

I have pushed a new version of my branch that I hope is good enough
for review. I have a couple of things in there which I'll describe
below.

Firstly, I wonder if you feel comfortable diffing all these changes at
once and cherry-pick? Or if you'd prefer that I did one thing at a
time and passed it to you for review.

My desire is of course to contribute it all if you'll see it fit, but
since I want to use the real Fabric and not my fork (we'll be a bunch
of developers using the tool), I'd rather get off my fork and onto
your master (and upcoming new release ;) ) soon enough.


On Mon, Oct 20, 2008 at 6:26 PM, Christian Vest Hansen
<address@hidden> wrote:

>> Here's what I've continued with:
>>
>> On Sun, Oct 19, 2008 at 4:18 AM, Christian Vest Hansen
>> <address@hidden> wrote:
>> I understand your concerns with what I did with prompt(). I'm after
>> three things:
>>
>> 1. To (always) repeat the prompt if validate fails. I skipped the
>> 'repeat' flag in my new version.
>>
>> 2. To invoke a given validate even if we have a supplied value. I just
>> removed my 'auto' keyword and changed the use of a present varname in
>> ENV so that this gets validated as well..
>>
>> 3. I'd like to use regexp validations, so I've kept the overloaded
>> 'validate' (and updated the docstring) which uses the RegexpValidator
>> (previously named Matcher). Do you think allowing regexp strings is a
>> bad idea? Perhaps we could keep RegexpValidator or similar, so at
>> least that one is available as a convenience tool? Though think I'd
>> prefer a less "obtrusive" helper..
>
> These three points make It _sound_ better.
>
> I don't know if Jeff has put any code towards any of his ideas
> regarding prompt, though I don't see anything in the commit log for
> his master branch.

Nice. But do you think my version of `prompt` is good enough, or
should I/we work with Jeff as well to arrive at something like this?


>>>> * improved "load":
>>>>    - will only invoke the loaded fab script once. Thus multi-file based fab
>>>>      setups can shared loaded scipts without having them invoked multiple
>>>>      times.

In addition to my last changes, `load` does now use execfile with a
restricted namespace (built from the registries - plus `var`, see
below). This way you can control what is accessible to the fabfiles,
and also pick from the result (using the `captured` dict).


>>>> * new operation "call_once":
>>>>    - will call the given function (command) *unless* it has already been
>>>>      called.
>>>
>>> These ideas I like. Especially the improvements to load.
>>
>> I have *preliminary* working versions of these. Especially call_once
>> needs more thinking though (regarding messages, and if we could reuse
>> the '_execute_commands' logic for parameters etc -- see my TODO
>> scribble).
>
> You can use pull-request (github feature) or send an email when you
> think it's ready.

I have completed this "to my own taste", but I'd like for your review.
`call_once`is now named `invoke`. The actual control of whether a
command has been executed is put in your _`execute_commands`
(actually, in the `_execute_command` I factored out). I did this since
I believe it makes sense to not invoke the same command twice if you
supply it on the command line and also reference it in `invoke` (or
@depends).

This should only change the current behaviour of Fabric in one way: if
you give the command name twice to "fab", it will only run once. If
you actually *want* to invoke a command twice or more, they can of
course still be called as regular functions from other commands.


>>> These would be the first decorators intended for use in fabfiles.
>>> You/we will have to think about how they are going to interoperate
>>> with the list and help commands.
>>
>> Absolutely. I've added these, without any documentation, and will
>> continue with how to register them and integrating that in the help
>> system. '@depends' will also be aligned with how I'll proceed with
>> improving 'call_once'.
>
> Sounds good :)

I have added decorators to the registry and help system now. (But see
my previous mail about the issues with `func_code`).


>>>> == After that ==
>>>>
>> Basically I just like to (as Jeff also seemed to) solve the issue that
>> 'set' currently shadows the builtin with the same name. We then toyed
>> with the idea of exposing one object to replace the get/set (which I
>> fully understand if you don't want to remove though for e.g.
>> compatability, clean names etc). Since 'vars' is also a builtin, I
>> thought of 'params'. Perhaps e.g. 'var' would be better.. Working
>> something like:
>>
>>    # Define one or more variables (just as with current "set"):
>>    var(fab_user='me')
>>    var(fab_hosts=[...], fab_user='me')
>>
>>    # Define one var can also be done like:
>>    var.fab_user = 'me'
>>    var['fab_user'] = 'me'
>>
>>    # For getting them, same pattern:
>>    print var.fab_user
>>    print var['fab_user']
>
> Looks nice. I think `var` is a good name. I wasn't totally happy with
> the shadowing when I introduced `set` but I couldn't come up with a
> better name.

I've implemented this as well, by turning `ENV` into an instance of
`Variables`, which is a subclass of dict. (And prepopulated with
`DEFAULT_ENV`.) It is passed to the fabfiles under the name `var` via
execfile, as I described above.


Again, I hope you'll find at least parts good enough to merge -- and
if not, please instruct me with any fixes you'd desire.

Best regards,
Niklas