pijul unrecord would display a list like e.g. push does, allowing you to select changes to unrecord.

This is tricky. What do you think should happen if there are 300_000 changes on the branch?

Hm, good question. Could we display the last, say, 20 changes?

Of the two changes I just pushed, I wanted W5 to depend on RM, but I couldn’t get it to work. In any case, I implemented my suggestion of showing 20 changes to unrecord. The way I did it is obviously horrible—I don’t know how to look up all the necessary information from a hash, so I turn the hashes I already have back into strings and look them up with hash_from_prefix.

It’s basic, but it works. I noticed a weird usability problem though—I was intuitively selecting the changes I wanted to keep and deleting the ones I wanted to unrecord.

Good work, although the choice of 20 is somewhat arbitrary. Would you care to add a command-line flag?

Hm, that would be a flag that only applies to pijul unrecord without any arguments. What do you think about this being a configuration setting?

I did it with a config setting for now, what do you think? I’m not sure I’m 100% happy with it myself.

Sorry for not answering sooner. I’m ok with either, and having both is even better in my opinion.

Correct me if I’m wrong, but it seems #PN4J4NYNZTX56DDTMAWZBFM75QOH3SDVXPADM4PMCPRKJKEHL35AC has a lot of unwanted stuff (zombie line resurrections), possibly because make_change_list now lives in pijul/src/commands/mod.rs.

I re-added the change with the config option without the extra stuff and also added a command line option. No default is assumed, i.e., if the config option is not present and the command line option is not passed, that’s an error.

I also took the liberty of improving the help messages for unrecord a bit. It’s a separate change in case it’s not wanted.

Hi! Sorry for letting you wait on this. It seems there are conflicts with some changes on main, would you mind either fixing them, and possibly re-record a single change to push to this discussion?

Thanks.

No problem, here’s everything in one change :)

Thanks! Just sent it to CI.

It still contains my kludge of running hashes that we already know for a fact to be valid through hash_from_prefix again. That should probably be fixed, but I don’t know how.

Coming back to this now, I’m not sure I see what you meant in your last comment. I don’t think I touched that file much since your change, so the issue is probably still there.

I added a review that hopefully explains the issue.

Reviews don’t work yet, unfortunately, meaning that your review was recorded, but it is marked as a draft, until you click the “publish” button… which I forgot to add to the page.

I’m sorry about that. Can you explain the issue here, by copy-pasting code blocks?

Sure, no problem.

let hashes = txn
    .reverse_log(&channel.borrow(), None)
    .map(|(_, (h, _))| h)
    .take(number_of_changes)
    .collect::>();
let o = make_changelist(&repo.changes, &hashes, "unrecord")?;
parse_changelist(&edit::edit_bytes(&o[..])?)
    .iter()
    .map(|h| h.to_base32())
    .collect::<_>()

This is where we obtain the list of hashes to unrecord. Note that we take the hashes that are returned by parse_changelist and save only their base32 representations.

let mut changes = Vec::new();
for c in hashes {
    let (hash, change_id) = txn.hash_from_prefix(&c)?;
    let n = txn
        .get_changeset(&channel_.changes, change_id, None)
        .unwrap();
    changes.push((hash, change_id, n, c));
}

Then, later, we take the hashes (or prefixes) and look them up with hash_from_prefix. In the case that the hashes were given as command line arguments, this is of course necessary, but if we obtained the hashes from parse_changelist in the first place, this is needlessly roundabout. The problem is that I don’t know how to get the change_id without invoking hash_from_prefix.

I see. What you wanted is txn.get_internal and txn.get_external, which are part of the libpijul::GraphTxnT trait. I’ve just handled this, and I believe we can close this discussion (feel free to reopen if you disagree, of course).

Thanks for handling this, I wouldn’t have found this.