On Tue, Sep 04, 2007 at 09:52:06PM +1000, William Uther wrote: > On 04/09/2007, at 6:45 PM, Nathaniel Smith wrote: > >On Thu, Jul 05, 2007 at 03:09:04PM -0700, William Uther wrote: > >> register_command(command, abstract, description, lua_function) : > >>Adds a new command to the "user" group of commands. When the command > >>is called, the associated lua function is executed. That lua will > >>normally use mtn_automate() calls to do its work. > >> > >>I also added automate versions of the following normal commands: > >>push, pull, sync, merge, update and commit. These are currently not > >>very 'automate' friendly in that they still output everything to > >>standard out. They work for this purpose though. > > > >The requirement for landing a new automate command in mainline is that > >the semantics (what it does) and interface (how you tell it what to do > >and how it tells you what its done) must be fully documented and > >tested. Also, we should be willing to keep those semantics and > >interface fixed going forward. Having automate access to the > >functionality you mention would indeed be nice; you can decide what > >the easiest way to get commands that expose that functionality and > >meet the bar for automate inclusion is. > > The documentation is not great. I have already merged with trunk. > > Would you like me to disapprove?
I tried to look at the documentation in trunk, but I can't find it at all. Is it even documented in monotone.texi? The --help docstrings are okay for just docstrings, but definitely not as actual documention. There is Massive Magic in update, commit, merge at least -- for instance proper documentation, by automate standards, would have to say something about how update determines what to update to (it is *not* "the head of the current branch"), how commit decides the what the commit message should be (this is like 20 lines of logic), and so on. The netsync commands probably aren't so bad, but even then there's all the stuff with the default sync target being magically read and sometimes magically set. Basically, the problem is that there's no-where near enough information here to write programs that talk to these commands and get reliable results. The only programs you can write are the ones who actually want the interface specification to be "do whatever you'd do if the user made some request directly in the user-level interface", i.e., programs that are just thin proxies between the user and mtn's normal interface. AFAICT there are also no tests for the new automate commands whatsoever? (I may have missed something.) Having no tests is unacceptable in general; having no tests and having no docs in the manual are both automatic reject-this-patch checkboxes for automate commands in particular; these particular commands seem to have some major problems being automate-friendly as well. Sorry to be so negative :-/. There is nothing wrong with writing programs that are thin wrappers around mtn's normal user interface, though, just we can't make the same kind of guarantees for those commands stability, so they need to be set apart from the rest of automate. Would it make people happier if we added an automate command 'mtn automate user <foo>' which does exactly the same thing as 'mtn <foo>' except with output redirection and it can be used over automate stdio? > The netsync operations still output their ticker to stdio. You can > turn it off with > the normal --ticker option. There isn't much other interface there. > > The other commands, update and commit, do have defined interfaces. > I'm not sure that > they're the best interfaces yet, and they are only defined in only defined in...? > >>I've also added contrib/extra-commands.lua which has the following: > > > >In principle I have no real problem with making monotone more > >user-extendable; it can be a great way to prototype new features, see > >how useful they are in the wild and whether they're worth folding into > >core, etc. > > That was my main aim. Cool. > >By the time we're actually be *shipping* these in our own source > >tarball, though, maybe we should be considering whether we're just > >adding these features as extensions because we don't have the communal > >will to make the program itself better. (See also > >http://ometer.com/free-software-ui.html) > > How can you see how useful something is in the wild without shipping it? By 'shipping' I mean in particular 'in the official release tarball'. Not that contrib/ is particularlly official, even if it is part of the official tarball. I'm not really objecting to the particular case, just raising a warning flag for us to keep in mind... > I also think the monotone interface needs work. I don't want to get > into > a big argument about it though. I'd rather propose solutions. > > However, I don't claim to be able to get it right first time either. > I want > an easy way to try a bunch of commands and see which work for me over > time. > It is an easy way to prototype. Cool. > My current set of commands is here: > > http://www.cse.unsw.edu.au/~willu/monotone-patches/mtnrc > > Note that I'm using alias_command for more than just blame and > praise. It also > allows you to name your commands properly and add short aliases just > like a C++ > command. Yes, if you can define new commands, you should definitely also be able to alias them. > Also note that I have error checking in there now. I just haven't > had time to > commit this yet. What's the interface like for getting errors out of mtn_automate()? > >It looks like in both of the examples, you actually don't want > >automate > >at all -- you're not doing anything programmatic against mtn's data > >model, you're just driving the user interface. > > Not entirely true. The remote_commit command gathers the number of > heads > and outputs a warning if there is more than one. It asks the user to > merge and check that the merged revision is ok before pushing. Okay, so you're doing some things programmatic, and some things driving the user interface. These are still different things :-). > >If what you want is to > >just run user commands, maybe it would be better to be running user > >commands than to first move them over to automate (in a sort of > >half-done way) and then use them via automate? > > There had been requests on the mailing list for a more complete > automate set. > I was also planning to add "add" and "drop", etc, but I haven't had > time. Those would be very cool indeed, though again the user commands have lots of weird and magic stuff in them, so we want to be careful to add things with reliable semantics. > >Another approach to consider might be to have lua commands that were > >run with very little mtn infrastructure baked in around them -- like > >add a way to use mtn as just a lua interpreter (one that is always > >available when mtn is), that has no additional fanciness except maybe > >scripts are told where the mtn binary is and what command line options > >they might want to pass. Then scripts could just shell out to mtn > >like any other app, and they could use the command line options thing > >to pass on -d and friends correctly. This would make writing such > >scripts somewhat more complicated -- you'd need the same shelling-out > >support code that other frontends to mtn need -- but that's code that > >only needs to be written once and then we have everyone working on > >improving one interface, instead of splitting development effort > >across two. > > That is possible. I didn't consider that. Would you prefer that > solution? I don't think it is better than what is there currently. The advantages would presumably be: -- somewhat less maintainence work, maybe -- you could call both automate and user commands with impunity. The latter is really the reason I was thinking of -- as you may have gathered, the tension between UI-level and engine-level interfaces seems pretty central to all of this stuff we're talking about to me :-). > The bottom line here: I've become pretty busy lately. I don't have time > to fine-tune this right now. If there are parts of this that are a > problem > merged into trunk, then I apologise - feel free to revert them. I guess for me there are two parts here: the new automate commands, and the extension command stuff. The former part I discussed above. The latter I don't have any major objection too, so long as it's documented and tested (it is documented and tested, right? I'm having a lot of trouble figuring out your changes, because somehow you seem to have not sent the .lua_cmds branch certs to monotone.ca? Anyway, experience has shown that documentation and tests get written before merging or they don't get written at all, so we try to be anal when we can). Could you revert the parts that don't meet this? Like I said, I'm having trouble tracking down what happened. -- Nathaniel -- Damn the Solar System. Bad light; planets too distant; pestered with comets; feeble contrivance; could make a better one myself. -- Lord Jeffrey _______________________________________________ Monotone-devel mailing list Monotone-devel@nongnu.org http://lists.nongnu.org/mailman/listinfo/monotone-devel