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

draft_commit_description template does not show branches on current change #4216

Open
gpanders opened this issue Aug 5, 2024 · 5 comments
Open
Labels
polish🪒🐃 Make existing features more convenient and more consistent

Comments

@gpanders
Copy link
Member

gpanders commented Aug 5, 2024

Description

I'd like to show branches pointing at the current change in the draft commit description template. The following template:

draft_commit_description = '''
concat(
  description,
  indent("JJ: ",
    concat(
      "\nBranches pointing to this commit:",
      indent("    ", branches.map(|b| b.name() ++ "\n")),
      "\n",
    ),
  ),
)
'''

shows only Branches pointing to this commit: with no branches listed, even when there are branches pointing at the current revision.

Steps to Reproduce the Problem

  1. Set the draft_commit_description template to the above value
  2. Create a new branch with jj branch create test
  3. Open a description message with jj describe

Expected Behavior

The draft description message should show:

JJ: Branches pointing to this commit:
JJ:     test

Actual Behavior

The draft description message shows:

JJ: Branches pointing to this commit:

Specifications

  • Platform: macOS
  • Version: jj 0.19.0-f4dd856f9f29556b09b904c27b7b2ebdd1fd060e

@martinvonz gave some context on Discord:

I think that's a bug in how draft_commit_description works on a temporary commit (we don't move branches over to that commit)

@martinvonz
Copy link
Member

@yuja: For describe, I suppose we could easily fix it by creating a whole temporary transaction instead of just a temporary commit.

I think the other commands (split and commit) are more interesting from a UX perspective. If there's a branch pointing to the commit you're splitting, would you expect branches to include that? It seems odd if the branch was included in the resolved template description for both commits after the split, but it seems reasonable if it's included for the second commit (because that's where we currently leave the branch after the split, but see #3419).

@yuja
Copy link
Contributor

yuja commented Aug 6, 2024

The easiest workaround I think is to add predecessors keyword. New commit isn't created yet at this point, so it's technically correct that the branches pointing to the predecessor aren't moved. (I agree it's a bit unintuitive, though.)

predecessors.map(|c| c.branches.map(..))

For split (and describe ancestor commits in general), I think user would usually want to see which branch the commit belongs to. We might need generic revset support to query descendant commits relative to templater's self.

@yuja
Copy link
Contributor

yuja commented Aug 6, 2024

generic revset support to query descendant commits relative to templater's self.

#3402 might also help. If topic is implemented as a commit metadata, it will be copied to a temporary commit. On split, both commits will have the same topic.

@martinvonz
Copy link
Member

The easiest workaround I think is to add predecessors keyword.

It seems even easier to create a temporary transaction that we then discard. Then we can move the branch in that transaction. Would that not work?

@yuja
Copy link
Contributor

yuja commented Aug 6, 2024

It seems even easier to create a temporary transaction that we then discard. Then we can move the branch in that transaction. Would that not work?

It will probably work, but I assume it would require more code duplicates. We'll basically need to run the same code path twice to get slightly different outcomes. I just meant a template keyword is easy to implement, not easy for use.

@PhilipMetzger PhilipMetzger added the polish🪒🐃 Make existing features more convenient and more consistent label Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
polish🪒🐃 Make existing features more convenient and more consistent
Projects
None yet
Development

No branches or pull requests

4 participants