I just noticed that:
I'm currently trying to implement it myself, so please wait for me.
Implemented 1. 2 will be trickier for me, but I'll try later.
Thanks for the patch, I'm going to apply it now.
There is currently no table of tags (we try to keep the number of tables as small as possible), so the best you can do for 2 is traverse the patches, collect the tags, and output that.
yes, that's what I planned to do! Thank'you for accepting the other patch, I need it for my gui viewer
EDIT: compiles fine, so if you like it, you can merge.
Regarding this second patch, I would rather use the occasion to create a new command show, with two subcommands, dependencies and tags (and why not branches?). We could deprecate show-dependencies.
(also, glad to see that having several branches in the repository now is useful :D)
@lthms Not sure about this (though definitely show-dependencies is very ugly).
pijul show --dependencies
While I'm in favor of merging patch and show-dependencies into show, I think that branches and tags belong each to their own subcommands
I don't know. pijul patch is actually used as-is for remote stuff, so we should probably let it at least for backward compatible reason (or brace ourselves for impact of curious users who cannot use the nest). Also, I like the subcommand approach better, but there is no rational here, so…
On a side note, I’ve reviewed your patch, and you should use the is_tag() function. Currently, there is only one flag defined, but this might change in the future, and if so, your patch won't have the expected behaviour.
is_patch is defined in libpijul/src/patch.rs:299, and is used in show-dependencies.
Fixed! I actually knew about is_tag(), but ended up not using it because it's only available for Patch, not PatchHeader, which I preferred because it seems it might be more performant (though I really don't know) and is used in pijul log.
I added a "--label-only" flag to only display the label.
is_tag could (should?) probably be implemented for PatchHeader too (and the is_tag of Patch would be a call to is_tag of PatchHeader). This way, you shouldn’t have to load the entire patch, and only the header would be sufficient.
Also, wouldn’t it be better to just use the same display than for pijul log? (and don't remember just now where it is, maybe in ask.rs)
(Sorry for the incremental review, I’m still new at this)
I implemented it that way because I understand tags as labels, not patches. But maybe now that we have --label-only I could implement the default using the same as log. What do you think?
(Yes, it's in ask.rs; it would be very easy to use that, I just thought it's not fitting for the purpose of this command)
I too believe that is_tag should be implemented for PathHeader, it makes more sense.
You know what? I'll implement it as you suggested. It's better. Only doubt: what should be the default view? compact or verbose?
Done! Let me know if there's something else to fix
Neat! I think we are almost good to go, I’d just call setup_pager once (before the for).
Ops, missed that =D Fixed now.
Let me try that this evening, just to be sure, and then I will happily apply your patch. Thanks a lot!
Some of the patches in this discussion conflict. @yory, could you please remove the extra ones?
Also, would you care to write one test (just to make sure our coverage doesn't decrease too much, these things are hard to get back up).
The extras should be gone now. It's only one patch: 6rWFPzF7AF3XXvUVXJpwG54TxYHnzy7LDmMBnichUNsT9jiTxqgynZ9xcJAmZafGnFWrGDjdW5PYsWWcPFDLqix8
As for the test, I'll see to it later, either today or tomorrow.
How can I tell bats that some parts of the output can change from run to run (in my case, the hash, internal id and timestamp)?
The way it's done in other tests is to parse the output of the command, the hash is printed.
I can't find how to make sed work. This command seems totally legit but doesn't work: pijul tags | sed -e 's/Hash: .*/Hash: <witheld>/g'
pijul tags | sed -e 's/Hash: .*/Hash: <witheld>/g'
I suspect it's because pijul is sending this:
Maybe use NOPAGER=1 pijul ...?
NOPAGER=1 pijul ...
All ready for merging.
Thanks a lot for your dedication. Unfortunately, I will not be able to review and apply your patch right now, but I will probably be able to do it this evening.
No probs, take your time!
The test fails in my case. I will try to have a closer look in order to understand why.
Strange! I ran it using pijul from master, with this command: ./run_tests.sh cases/tags.bats.
Maybe is the perl version (i'm using it for the regex)? mine is v5.26.2