unisonweb / unison

A friendly programming language from the future
https://unison-lang.org
Other
5.66k stars 266 forks source link

transcripts: project/branch should be validated? #5173

Open ceedubs opened 1 week ago

ceedubs commented 1 week ago

Is your feature request related to a problem? Please describe.

I think that the current transcript behavior in ucm blocks is that a line like project/branch> view foo causes ucm to implicitly use the project/branch namespace without an explicit switch. This probably makes sense for the initial ucm command in a transcript, as you need to start somewhere.

However, I think that the implicit switching and project creation makes it awkward to use transcripts to test certain functionality. For example, imagine that you want to test that project.rename keeps you in the same branch after the rename. You might write something like this:

proj/my-feature> project.rename proj2
proj2/my-feature> ls

But this won't really assert that project.rename kept you in the /my-feature branch, because leading the line with proj2/my-feature will implicitly switch you to the my-feature branch even if an interactive ucm session wouldn't.

Describe the solution you'd like

I think that it's fine for the initial ucm command in a transcript to implicitly declare a project. But for all subsequent commands I think that it might make more sense to make the project/branch specifier an assertion to validate, and that switching projects/branches should require an explicit switch in a transcript just as it does in ucm.

The more that the transcript and interactive session behavior diverge, the less helpful transcripts are in detecting bugs.

Describe alternatives you've considered

I haven't thought a lot about alternatives. But I think that there should be a pretty compelling reason for each divergence of behavior between transcripts and actual ucm sessions.

Additional context

This also came up here when there wasn't a way to add a regression test for remembering the active branch in a project.

mitchellwrosen commented 1 week ago

For historical context, we've always had a transcript parser that let you do things like

.> whatver
.a> whatever

without an explicit cd. When we added projects, we also added implicit switch behavior matching that.

And then very recently, we even allowed you to create a new branch just by mentioning its name in a prompt! So, the implicit magic / diverging from a real UCM session is worse than you describe in the ticket. And it's true that certain things are impossible to test in transcripts with the auto-switch and auto-branch features. That's not great.

I could personally go either way here, FWIW. Transcripts are quicker than ever to write now, which matters a little.

aryairani commented 1 week ago

Related: I was thinking recently that scratch/main> is a lot to type to start each and every line in the transcript.

So how about something like:

  1. > defaults to the "current branch", which is scratch/main initially, but can be altered by other commands. the actual branch is shown in the output.md
  2. /branch or proj/branch> implicitly creates-empty and switches to /branch or proj/branch respectively; this rule can be kept or we can get rid of it
  3. > or /branch or project/branch> alone on a line is valid; the current branch or specified branch is shown in the output.md as well

e.g.


Input:

```ucm
> ls
> branch.create foo
>
/bar>
other/other>

Output:
scratch/main> ls

  nothing to show

scratch/main> branch.create foo

  Done. I've created the foo branch based off of main.

  Tip: To merge your work back into the main branch, first `switch /main` then `merge /foo`.

scratch/foo>
scratch/bar>
other/other>


4. Currently we suppress the normal implicit branch creation output, but we could decide to include it in the output
ceedubs commented 1 week ago

@aryairani I think that my honest answer is that saving a bit of time copy/pasting the prompt from the line above doesn't seem worth it to me. I think that it's handy to have transcripts that show user exactly what a session would look like. And also the more special-cased logic that they have, the more chance that they don't emulate actual ucm behavior (and therefore don't catch a bug).

I do write a decent number of transcripts for bug reports, but admittedly I don't write as many as people who work on ucm. So I might just not feel the pain of writing prompts in transcripts enough. I'm not strongly opposed to the proposal; I just question whether the additional complexity holds its weight.

aryairani commented 1 week ago

@ceedubs To clarify, the output is still supposed to show what the session would look like; the input never has. Not sure if this changes your answer.

mitchellwrosen commented 1 week ago

Related: I was thinking recently that scratch/main> is a lot to type to start each and every line in the transcript.

I don't think this is sufficient justification for adding more features to the transcript parser – the less code the better (of course) – and a little copy+pasting of prompts while writing transcripts isn't so bad :)