Junio C Hamano <gitster@xxxxxxxxx> writes: > Karthik Nayak <karthik.188@xxxxxxxxx> writes: > >> Let's say a user is iterating with a prefix set to 'refs/heads/', this >> would iterate over all the refs with that prefix. But mid-way the user >> realizes that they only care about 'refs/heads/feature/' prefix and they >> ask the iterator to set that as the prefix. >> >> In such a situation, the iterator seeks to 'refs/heads/feature/' and >> will only yield references with that prefix. In short, the previous >> prefix state was reset. > > Yes, even though I wouldn't call such an operation "seek", "Ah, I do > not need the entire refs/heads/ walked, only refs/heads/feature/ is > enough" is an operation mode that makes sense. > > But not for paging, though. > > If your web application is showing all branches, one pageful at a > time, and the first page ended at refs/heads/feature/something and > you ended up "seeking" to refs/heads/feature/ to start the second > page, you do not want your second page to end when the iteration > goes out of refs/heads/feature/ hierarchy, no? > Yup and this (we show all references beyond the seek) is the current implementation. I was talking about the internal implementation of 'refs_iteration_seek()' which is the function used for seek and setting the prefix. To clarify, this is the current implementation: $ git for-each-ref 6f4b58c4968eb82277cf5b1cf8775117e5b83de0 commit refs/heads/bar 6f4b58c4968eb82277cf5b1cf8775117e5b83de0 commit refs/heads/feature/x 6f4b58c4968eb82277cf5b1cf8775117e5b83de0 commit refs/heads/feature/y 6f4b58c4968eb82277cf5b1cf8775117e5b83de0 commit refs/heads/foo 6f4b58c4968eb82277cf5b1cf8775117e5b83de0 commit refs/heads/goo/x 6f4b58c4968eb82277cf5b1cf8775117e5b83de0 commit refs/heads/goo/y 6f4b58c4968eb82277cf5b1cf8775117e5b83de0 commit refs/heads/master 6f4b58c4968eb82277cf5b1cf8775117e5b83de0 commit refs/tags/tagged/2 6f4b58c4968eb82277cf5b1cf8775117e5b83de0 commit refs/tags/tagged/3 6f4b58c4968eb82277cf5b1cf8775117e5b83de0 commit refs/tags/v1 6f4b58c4968eb82277cf5b1cf8775117e5b83de0 commit refs/tags/v2 $ git for-each-ref --format="%(refname)" --start-after=refs/heads/goo refs/heads/goo/x refs/heads/goo/y refs/heads/master refs/tags/tagged/2 refs/tags/tagged/3 refs/tags/v1 refs/tags/v2 $ git for-each-ref --format="%(refname)" --start-after=refs/heads/master refs/tags/tagged/2 refs/tags/tagged/3 refs/tags/v1 refs/tags/v2 $ git for-each-ref --format="%(refname)" --start-after=refs/heads/goo/x refs/heads/goo/y refs/heads/master refs/tags/tagged/2 refs/tags/tagged/3 refs/tags/v1 refs/tags/v2 $ git for-each-ref --format="%(refname)" refs/heads/feature refs/heads/feature/x refs/heads/feature/y You can see we list all references beyond the seek. > It seems to me that the root cause of the confusion is because > prefix, which is to let iteration finish way before the data runs > out (instead finish when the iteration steps out of a given > subhierarchy denoted by the prefix), is somehow abused as the > current position of the cursor. Shouldn't they be two separate > concepts? The cursor needs to fall within the prefix while the > iterator is active, so they are not two totally independent things, > but prefix is pretty much static while the cursor position is very > dynamic. > The prefix setup in 'ref_iteration_seek' does two things, let's consider prefix: 'refs/heads/feature' 1. It sets the cursor to seek to 'refs/heads/feature' 2. It also sets the internal prefix matching to 'refs/heads/feature' In Contrast seeking via 'ref_iteration_seek' only sets the cursor to 'refs/heads/feature'. To make this simpler, we've changed 'ref_iteration_seek' to do: 1. seek the cursor to the requested reference 2. Set prefix if the REF_ITERATOR_SEEK_SET_PREFIX is set, and unset the prefix otherwise. The state reset I was talking about in my previous emails refers to step #2 here, where when no 'REF_ITERATOR_SEEK_SET_PREFIX' is set, we remove any previous prefix set. >> This series did start out that way around, so ease of implementation >> isn't it. It was more of a side-effect of not clearing state. > > I am even more worried about usability and correctness aspect of > what was described here now. After seeking to refs/heads/feature/, > do we continue to iterate and step out of refs/heads/feature/ > hierarchy or can we cut off a particular page that started with a > ref within refs/heads/feature/ subhierarchy when we exhaust refs in > refs/heads/feature/ and have to wait for getting asked for the next > page before we show refs/heads/gsomething that is outside > refs/heads/feature/ and sorts after? The "I reset to iterate over > refs/heads/feature/ because the entire refs/heads/ is not what I > care about" example makes me worried about this. > > Thanks. I think we're crossing paths and talking different things. I hope the examples above clarify things. The current implementation doesn't support '--start-after' and prefix setting at the same time: $ git for-each-ref --format="%(refname)" --start-after=refs/heads/master refs/heads fatal: cannot use --start-after with patterns Happy to clarify if this doesn't make sense. Thanks
Attachment:
signature.asc
Description: PGP signature
