GitoxideLabs / gitoxide

An idiomatic, lean, fast & safe pure Rust implementation of Git
Apache License 2.0
8.91k stars 303 forks source link

Improve help text about hidden repos #1471

Closed EliahKagan closed 2 months ago

EliahKagan commented 2 months ago

Note: This PR is either mostly or completely was in substantial part superseded by #1470 as discussed in #1469, and has been narrowed in scope. The following description does not reflect that.

Changes to the warning message

In the first commit, this unconditionally augments the warning suggesting to use --skip-hidden-repositories so that it mentions worktrees as well. It shows that slightly longer message even in a repository that has no worktrees. This has the benefit of simplicity, is the requested change in https://github.com/Byron/gitoxide/issues/1469#issuecomment-2252163765 if I understand properly, and makes sense as an initial mitigation for #1469.

The message is not much longer, and if that level of detail is acceptable, it might even be okay to have it like this longer term, though I suspect it might not be necessary given other forthcoming changes.

I considered parenthesizing "and worktrees" but it looked to like "repositories (and worktrees)" could be misread as "repositories (and their worktrees)" to mean the same thing as "repositories."

I did not make a corresponding change in the other similarly worded warning message suggesting to use --find-untracked-repositories, because there does not appear to be an analogous situation with untracked directories where a worktree of the current repository would be removed. I did some testing to check this.

Changes to the help text

It seemed like the right time to also propose changes to the help text for gix clean options directly relevant to the deletion of nested repositories, since I was already planning to do that for -r. In the second commit, this does that, at least an an initial wording that might be possible to improve on, and also applies an analogous change--but with more details, since this is help text--to the change in the warning message.

I made them separate commits so it's easy to take only one (I imagine it would be the first) if not all these changes are wanted at this time.

Commit messages

Even though this only adjusts messages, from this guideline it seems like the commits should have fix: prefixes, so I did that. I'm not sure if that's right. This PR does not carry any behavioral change other than the text of messages that are displayed.

EliahKagan commented 2 months ago

Sounds good, I'll have a commit that removes the mention of worktrees in a couple of minutes.

EliahKagan commented 2 months ago

I've done that in fcebc44. Because of the way it was done, all three of the commits here are prefixed fix:. Given the effect on the automatically generated release notes, maybe this is not justified. Perhaps a rebase should be done to edit the messages or to squash the commits into one. Squashing seems reasonable since the actual change here is quite minimal; that this is more than one commit is solely a result of the process that led to it.

Byron commented 2 months ago

I think it's fair to use fix: three times if adding three messages is intended or at least makes sense to you. It's definitely your choice on how to design this.

EliahKagan commented 2 months ago

I think it's fair to use fix: three times if adding three messages is intended or at least makes sense to you. It's definitely your choice on how to design this.

Each one was something I intended at the time given what had come before, but none of these messages are really ideal now -- they basically all have to be read together.

On the other hand, they do make clear what the progression was in relation to the changes from #1470.

Maybe the best thing to do would be if I were to rebase this to edit the message in the first commit, 7a4d239, to remove fix:. I think the other two make sense together and they avoid claiming a change across versions that is actually absent. Would you be okay with that? Edit: Sorry, the "definitely your choice" wording in your comment actually answers that, and it can be undone if you decide it's not good. I'll push this change.

Byron commented 2 months ago

Thanks again for all the great work!