You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Näslund <da...@longitudo.com> on 2010/07/14 12:44:23 UTC

What's the responsibility of parse_next_hunk()?

Hi!

I just realized that parse_next_hunk() at present only detects a property
hunk if it has a property header ('Added', 'Deleted', 'Modified')
directly preceeding it. The second hunk won't know what propname it
belongs to.

We previously had diff headers and hunk headers. The property header is
only set before the first hunk and is thus not the same sort as any of
the two previous ones.

What should parse_next_hunk() do to properly handle property hunks?
---------------------------------------------------------------------
  Alternative 1
  Fetch the next hunk and tell the caller if it's a property or a text
  hunk.  (let it be up to the caller to remember what the last prop_name
  was)

  Alternative 2
  Keep track of if we're currently below a property header (and hasn't
  seen any text hunks since that property header). That means passing
  around a 'prop_name' variable that would be set to NULL if the previous hunk
  was a text hunk and otherwise have the value of the last parsed
  property name.

Any suggestions?

Thanks,
Daniel

Re: What's the responsibility of parse_next_hunk()?

Posted by Stefan Sperling <st...@elego.de>.
On Wed, Jul 14, 2010 at 02:44:23PM +0200, Daniel Näslund wrote:
> Hi!
> 
> I just realized that parse_next_hunk() at present only detects a property
> hunk if it has a property header ('Added', 'Deleted', 'Modified')
> directly preceeding it. The second hunk won't know what propname it
> belongs to.
> 
> We previously had diff headers and hunk headers. The property header is
> only set before the first hunk and is thus not the same sort as any of
> the two previous ones.
> 
> What should parse_next_hunk() do to properly handle property hunks?
> ---------------------------------------------------------------------
>   Alternative 1
>   Fetch the next hunk and tell the caller if it's a property or a text
>   hunk.  (let it be up to the caller to remember what the last prop_name
>   was)

I think 1 sounds better. It seems better to let parse_next_hunk() parse
hunks, and let the caller worry about additional context.

> 
>   Alternative 2
>   Keep track of if we're currently below a property header (and hasn't
>   seen any text hunks since that property header). That means passing
>   around a 'prop_name' variable that would be set to NULL if the previous hunk
>   was a text hunk and otherwise have the value of the last parsed
>   property name.
> 
> Any suggestions?
> 
> Thanks,
> Daniel

Re: What's the responsibility of parse_next_hunk()?

Posted by Stefan Sperling <st...@elego.de>.
On Wed, Jul 14, 2010 at 02:44:23PM +0200, Daniel Näslund wrote:
> Hi!
> 
> I just realized that parse_next_hunk() at present only detects a property
> hunk if it has a property header ('Added', 'Deleted', 'Modified')
> directly preceeding it. The second hunk won't know what propname it
> belongs to.
> 
> We previously had diff headers and hunk headers. The property header is
> only set before the first hunk and is thus not the same sort as any of
> the two previous ones.
> 
> What should parse_next_hunk() do to properly handle property hunks?
> ---------------------------------------------------------------------
>   Alternative 1
>   Fetch the next hunk and tell the caller if it's a property or a text
>   hunk.  (let it be up to the caller to remember what the last prop_name
>   was)

I think 1 sounds better. It seems better to let parse_next_hunk() parse
hunks, and let the caller worry about additional context.

> 
>   Alternative 2
>   Keep track of if we're currently below a property header (and hasn't
>   seen any text hunks since that property header). That means passing
>   around a 'prop_name' variable that would be set to NULL if the previous hunk
>   was a text hunk and otherwise have the value of the last parsed
>   property name.
> 
> Any suggestions?
> 
> Thanks,
> Daniel