Subject: bug#33167: 26; Doc string of `region-extract-function'

From: Eli Zaretskii <[email-masked]>
Date: Sat, 27 Oct 2018 19:32:45 +0300

> Date: Sat, 27 Oct 2018 08:27:41 -0700 (PDT)
> From: Drew Adams <[email-masked]>
> Cc: [email-masked]
> 
> > > 1. What are the BEG and END args passed to `filter-buffer-substring'?
> > >    Is BEG the smallest car of any of the zones in the noncontiguous
> > >    region, and END the largest cdr of any of the zones?
> > 
> > They are the first and the last positions of the region for
> > filter-buffer-substring to act upon.  That function is not supposed to
> > process non-contiguous regions.
> 
> Yes, but where do they come from, for that call?  How do they
> relate to, or how are they derived from, the noncontiguous
> region? Are they point and mark?  Something else?  That's the
> question.

It's entirely up to the region-extract-function that handles such
regions.  The default one doesn't.  So I don't see why these questions
need to be answered in the doc string of filter-buffer-substring, they
don't belong there and would only confuse.

> The only arg to the function that is the value of
> `region-extract-function` is METHOD.  The "region's content"
> is presumably the content of the noncontiguous region.  And
> presumably it is that that is "the content" that is returned
> as a string.  How is that string derived from the noncontiguous
> region?  That's the question.

It isn't.  The region passed to filter-buffer-substring must always be
contiguous.

> > > 2. `filter-buffer-substring' calls the value of
> > >    `filter-buffer-substring-function' with the same 3 args.  But what
> > >    can that function do with BEG and END (which are what?)?  It's
> > >    presumably a function that expects to use a single stretch of
> > >    buffer text from BEG to END.  But here we're talking about a 
> > >    noncontiguous
> > 
> > No, we are talking about contiguous regions when this function is
> > concerned.
> 
> Sorry, but I don't understand.  What contiguous regions
> are we talking about?  How/where in this doc did you
> get from a noncontiguous region to contiguous regions?

That depends on the implementation of region-extract-function that
supports non-contiguous regions: it must somehow call
filter-buffer-substring-function in a way that this function gets only
contiguous regions of text.  So this question cannot be answered in
the doc string of filter-buffer-substring-function.

> I haven't see your update to the doc string, but so far
> your description here doesn't give the impression that
> these questions have been cleared up.

Well, I suggest to read that first:

  http://git.savannah.gnu.org/cgit/emacs.git/commit/?h=emacs-26&id=df64da8eb845c9f07ee93bfbf28af41a01a2e83f

> > > 3. The 3rd arg to `filter-buffer-substring' just deletes the region
> > >    from BEG to END if it is non-nil, so it seems like passing that
> > >    non-nil 3rd arg is useless, as the region gets deleted anyway, by
> > >    `region-extract-function'.
> > 
> > Not sure what is the problem here.
> 
> Not sure what you're saying.

You say that passing the 3rd arg is "useless", and I don't understand
why.

> Are you saying that I'm wrong that a non-nil 3rd arg just deletes
> the region?  Or are you saying that I'm wrong that that non-nil 3rd
> arg is useless because the region is deleted anyway?

I'm saying that I don't understand what you wanted to say here.  At
all.

> > > 4. The use of `filter-buffer-substring' is also unclear.  It is passed
> > >    BEG and END (and METHOD, but see #3, above).  And it filters the
> > >    buffer text between BEG and END.  But see #1 above - are BEG and
> > >    END buffer positions that make sense for the whole region text?
> > >    Just what happens here?
> > 
> > See above.  It is not the job of filter-buffer-substring to DTRT when
> > the region is non-contiguous, it is the job of its callers.  See
> > rect.el for one example.
> 
> See above.

See above.

> What are BEG and END that get passed to it?

The doc string already tells that: the beginning and the end of the
region to be filtered.

> The doc should be able to make clear what the behavior
> is, without someone needing to investigate the code of
> rect.el.

It does.  What it does NOT have to do is tell how one would use this
function while implementing a replacement for the default
region-extract-function that can handle non-contiguous regions.

> Plus, noncontiguous regions are more general than rectangles.

Exactly.  So each such implementation has to figure out how to call
filter-buffer-substring in a way that makes sense and passes only
contiguous regions to filter-buffer-substring.

> The doc should make clear what happens in the general case.

The doc string of filter-buffer-substring needs to tell what that
function does.  And it does precisely that.

> The doc should make clear in general terms what extracting
> a noncontiguous region is about, and just what the value of
> `region-extract-function' does: what its arguments are and
> what its return value is (as a function of those arguments).

It does now, please see the new doc string.

> I did what you suggest before filing the bug report:
> checked all of the existing occurrences.  I felt that
> the doc is wrong wrt what really happens, and I feel
> that there's a need for a general description of this
> variable which helps.

The doc string was indeed incomplete before my changes; it is IMO
complete now.

> > > Or is what happens perhaps that EACH element of the noncontiguous
> > > region, that is, each zone (BEG<N> . END<N>) of the list ((BEG1 .
> > > END1) ...) gets filtered by `filter-buffer-substring', passing
> > > its BEG END and METHOD
> > 
> > Yes!
> 
> Then please say that in the doc.

NO!  Because that's just one possible implementation, for one possible
kind of non-contiguous regions.

> > > - so that a mapcar is applied?
> > 
> > Not literally, but the result is a list, yes.
> 
> If you haven't already done so, please say that in the doc.

If you haven't already done so, please read the new doc string.  I
think I covered everything that should be covered.