On Mon, Oct 11, 2021 at 7:21 AM Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> wrote: > > On Mon, Oct 11 2021, Carlo Marcelo Arenas Belón wrote: > > > [1] https://lore.kernel.org/git/20211011114723.204-1-carenas@xxxxxxxxx/ > > > > Carlo Marcelo Arenas Belón (4): > > blame: prefer null_sha1 over nullid and retire later > > rename all *_sha1 variables and make null_oid hash aware > > expand regexp matching an oid to be hash agnostic > > track oid_size to allow for checks that are hash agnostic > > > > git-gui.sh | 30 ++++++++++++++++-------------- > > lib/blame.tcl | 18 +++++++++--------- > > lib/checkout_op.tcl | 4 ++-- > > lib/choose_repository.tcl | 2 +- > > lib/commit.tcl | 3 ++- > > lib/remote_branch_delete.tcl | 2 +- > > 6 files changed, 31 insertions(+), 28 deletions(-) > > There was a similar series earlier this year which didn't make it that > fixes some of the same issues: > https://lore.kernel.org/git/pull.979.git.1623687519832.gitgitgadget@xxxxxxxxx/ This specific series is for git-gui, and the one posted before is for gitk, but the code is still similar enough, and indeed the gitk part was included in a reference. > Just seems like a lot of needless work as opposed to just matching > x{40,64} or whatever. Yes that's not the same regex semantically, but I > think the current code is just being overly strict, i.e. it's parsing > some plumbing output, we can trust that the thing that looks like the > OID in that position is the OID. > > If anything I'd think we could just match [0-9a-f]{4,} in most/all of > these cases, would make things like this easier to read: It makes me nervous though to see checks like the one I fixed on commit[1] that use logic to check the correct size of the SHA as an implication of it being a valid value. considering the code is very old, maybe that was relevant long ago?, but agree some checks seem to be unnecessarily strict. I have relaxed some of the checks in the gitk patch and will be posting it soon, so hopefully reviews from people that know the code better could be collected. Carlo [1] https://lore.kernel.org/git/20211011121757.627-5-carenas@xxxxxxxxx/
