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

idea: stg refresh demystifying what will be refreshed #180

Open
NonLogicalDev opened this issue Feb 4, 2022 · 4 comments
Open

idea: stg refresh demystifying what will be refreshed #180

NonLogicalDev opened this issue Feb 4, 2022 · 4 comments

Comments

@NonLogicalDev
Copy link
Contributor

NonLogicalDev commented Feb 4, 2022

The fact that stg refresh has subtle differences to git is fine, but it is also sometimes difficult to reason about. And leads to user confusion like [#161].

I have myself often ran into issues when I accidentally refreshed something into the patch I did not mean to. I have created helper scripts to wrap operations:

  • stg refresh --index
  • stg refresh

That show the changes that will be applied to the current patch. They are not bulletproof but they work most of the time:

https://gist.github.com/NonLogicalDev/8b2d77563688f6495f233704e5edd947#file-stg-refresh-utils-zsh

The result looks kind of like this:

Screen Recording 2022-02-04 at 1 16 23 PM

I wonder if we could add --preview-diff to the stg refresh operations to allow the user to verify the changes that are about to be made to the patch.

@NonLogicalDev NonLogicalDev changed the title idea: stg refresh demystifying what will refreshed idea: stg refresh demystifying what will be refreshed Feb 4, 2022
@jpgrayson
Copy link
Collaborator

My initial reactions to this are:

  • I love that you are thinking about how to make StGit's UI better! Thank you for taking the time to do this.
  • Doesn't running stg diff and/or stg diff --stat provide a sufficient preview?
  • Would my suggestion from stg refresh without staged changes #161 to make stg refresh -i fail if there are no staged changes help with this?
  • Seems like functionality like this might be exist in contrib versus being baked into stg refresh.

@NonLogicalDev
Copy link
Contributor Author

NonLogicalDev commented Feb 7, 2022

Hey @jpgrayson thank you for going over this! Your points are mostly valid but I wanted to give my perspective on the second one:

Doesn't running stg diff and/or stg diff --stat provide a sufficient preview?

The slight trouble here is that stg refresh is a rather complicated command with lots of options and depending on which options are applied you get drastically different results. The quick rundown of the most common are:

  1. whether user specified file filter via -- [FILES...]
  2. whether we are refreshing from index --index
  3. whether the user specified --spill (although that is mostly self explanatory)
  4. (have not used but) whether --submodules are specified

Previewing 1. can be attained via git --no-pager diff --stat HEAD -- [FILES...]

  • gives you a full diff + stat of difference between the worktree and HEAD (ignoring index)
  • Thank you for reminding me of stg diff that would work better for this case (though it does not allow suppressing pager, unless Git can do it via Env vars)

Previewing 2. can be attained via git --no-pager diff --stat --cached

  • gives you a diff between the index and the HEAD

stg diff covers only one of the scenarios. Overall this idea came from being always a little unsure if the diffs that are produced from the previous commands are what will be applied to the patch. (and from the need to know those commands, which newcomers probably don't)

@NonLogicalDev
Copy link
Contributor Author

NonLogicalDev commented Feb 7, 2022

I do somewhat agree that contrib could be a good alternative, but in an ideal world it would be nice to have a refresh --interactive like option that gives you time to understand what changes you are about to make to the patch before approving it, or realizing you meant do to --index instead of plain refresh, or you meant to apply the changes to a different patch.

But implementing that means implementing console UI which would further bloat stg, so to make the job of contrib script simpler it could be nice to allow a dry run of refresh with a diff output, that contrib tool could then use the diff output to show the user what they are about to do and verify that it is what they want.

@jpgrayson
Copy link
Collaborator

implementing that means implementing console UI

Yes, that's one of my concerns. Currently StGit does all of it's "interactive" business indirectly via the user's editor. Adding full-fledged console UI features to StGit would be a Whole New Thing. I'm not categorically opposed to having such interfaces baked into StGit, but I would want to have a bit of a comprehensive gameplan for console UI features and not just add a one-off.

it could be nice to allow a dry run of refresh with a diff output, that contrib tool could then use the diff output to show the user what they are about to do and verify that it is what they want.

I'm thinking about what a stg refresh --dry-run ... might look like or do. The main output you want is the diff/diffstat that will be recorded in the target patch. Yet another thing --dry-run could do is check whether the diff will trivially apply to the target patch. This doesn't matter when refreshing the top patch, but for any other patch there is no guarantee that the refresh will be conflict free. Yet another step beyond that would be to also simulate the reapplication of intervening patches to see if any of them may end with conflicts.

Maybe there are two features here: --preview which just provides the diff, and --dry-run which actually performs a dry run of the refresh and patch applications.

I should say that I generally like the idea of adding non-interactive capabilities to refresh, diff, etc. that enable a contrib script to orchestrate the interactive piece.

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

No branches or pull requests

2 participants