cvogt / cbt

CBT - fun, fast, intuitive, compositional, statically checked builds written in Scala
Other
488 stars 60 forks source link

sbt-revolver like restart feature #523

Closed cvogt closed 7 years ago

cvogt commented 7 years ago

@gratner I believe you were interested in 0905177?

nafg commented 7 years ago

I wonder if it makes more sense to not have loop and restart separate words? I mean is restart ... "parameterizing" loop or is it really an alternative looping mode?

cvogt commented 7 years ago

restart could automatically include trigger looping and "direct" model as well to avoid problems I think. Could you expand on the parameterizing @nafg?

nafg commented 7 years ago

@cvogt what I meant to say is that the way I parse a command like cbt direct loop restart or cbt loop test is similar to how I parse something like sudo watch time timeout 1 sleep 30: sudo is a command that takes parameters, its parameters are the command time timeout 1 sleep 30, well watch is a command that takes parameters, its parameters are the command time timeout 1 sleep 30, and so on. Each of the commands to the right could be run on their own, and the command on the left runs them with some modification to their "mode." So to, direct is a subcommand that takes a further cbt subcommand, like loop restart, and again, loop is a cbt subcommand that takes a further cbt subcommand, like restart. That's how the proposed syntax "reads" to me, and my argument was that that doesn't really express what's happening. loop isn't "running restart in a modified mode" akin to sudo or watch, in my understanding -- it's really that we're doing an alternative loop mode: The existing loop mode waits for source changes after its subcommand runs to completion, while the proposal is essentially to have a "looping" mode that waits for source changes immediately and then shuts down its subcommand and then runs it again.

I like the idea of shortening the whole thing to one word. After all since currently direct is the only mode supported, it may as well be implied.

While we're on the subject of looping syntax, to go on a bit of a tangent, have you considered instead of using loop doing what seems to be more standard, which is to have a -w / --watch flag (e.g. webpack and tsc, the typescript compiler, do that)? Or at least renaming it to watch instead of loop? It just seems to be the more common naming convention.

More relevantly, I don't know if we should spend the word "restart" on this. Let me explain.

Let's take a step back from "sbt-revolver" functionality and just question how web development would look without it. Would it make sense for cbt to the bash prompt while something is still running? E.g. in sbt with xsbt-web-plugin, you can run jetty:start and be back at the sbt shell, and later do jetty:stop (or just exit sbt). It might be a bad idea to return to bash something still running however. If so then how would one do two things at once (like ask cbt for the classpath while the webapp is running)? We could say they should use regular shell mechanisms, like appending & to background it, or running two terminals, but of course that's not ideal either. Also does the related project to "run a shell for subcommands of any command" have any bearing on this question?

In any case my point is, that if CBT would support running a webapp in the background, then you might want to have a restart command for that.

Actually here's an idea then... name the new command watch. I think loop has a stronger implication that it's "running something [to completion] in a loop [waiting for changes for each subsequent iteration]" while watch has more of a connotation of "being vigilantly on the lookup for activity [and taking action in response]".

(My understanding is that this can handle any subcommand.)

Thoughts?

cvogt commented 7 years ago

The existing loop mode waits for source changes after its subcommand runs to completion

correct

while the proposal is essentially to have a "looping" mode that waits for source changes immediately and then shuts down its subcommand and then runs it again.

incorrect. I tried that, but it doesn't work, because it only becomes apparent until after cbt's run which sources will need to be watched. Instead loop actually does the same thing and DOES take restart as an argument that it runs. restart is responsible for spawning the execution in a separate, detached thread and then exiting cbt as usual.

The only new thing is that restart will also write the pid of the process into a file and loop now kills all processes listed in that file before running the task again.

So it really works like your intuition reads the commands if I am not mistaken.

Not 100% sold on loop either, but following your reasoning about intuition watch compile would imply that it is watching compile not watching files. loop follows that inutition here that it loops compile (while also watching files as the trigger).

I like -watch. CBT has room to improve it's argument handling, which I haven't gotten to yet.

Regarding start and stop and restart. Right now killing the previous processes is implemented in the bash script, but maybe we should move that into stop? Then start could be what restart is now and restart could be stop; start. And looping is just separate from that.

cvogt commented 7 years ago

I think dropping to bash is fine even when a process keeps running and that solves the question about how to call other cbt tasks in the mean time