Re: [patch]:change, which allows to work with selections.

[Kai <kai willadsen gmail com>]

On 1 May 2011 08:40, Oleg LARIN <oleg larin st com> wrote:
> Hi, Kai.
>
> Some of my co-workers are using meld (as well as myself),
> all of us would like this feature and we had seen this
> implemented in other "merge" tools. This includes "Replace"
> functionality -  when user wants to copy over just some part of a chunk,
> lets say few lines instead of full chunk. This happens very often
> when somebody ports back newer software version to many versions
> backward - some of changes sitting together can not be applied to the older
> version as one chunk.
>
> That was why I have implemented this feature having also an idea not to change
> a lot.

I can appreciate the desire to keep the patch clean and simple, but I
think it's a more complicated change than that. I don't think you can
avoid implementing the selection-based actions separately to the
chunk-based actions.

> If you look at what this change does, it is exactly what you are saying -
> "I think we'd want to create a fake diff chunk from the current selection, and then
> call the existing chunk action methods with that".

It's true that that's what the change *does*, but I was referring to
how that behaviour should be implemented.

Among other things, your implementation will change the behaviour of
the central arrows, which I think is incorrect. If I click on one of
the central arrows, then I would argue that I'm definitely asking for
a chunk-based action, not a selection-based action.

Another point is that the behaviour is poorly defined if your
selection covers more than one chunk. What actually happens currently
is that the selection action will be performed relative to the chunk
the current cursor is in. I think in this case we should just refuse
to perform the selection-based action.

Also, my point concerning Delete remains; there's no reason at all to
change delete behaviour.

cheers,
Kai