search

martanne / vis

A vi-like editor based on Plan 9's structural regular expressions
Other
4.19k stars 259 forks source link

text-io: fix block shrinking when the recent piece is on the block's end #1126

Closed vkochan closed 6 months ago

vkochan commented 10 months ago

Looks like the original idea was to shrink the block if the text have been deleted on the recent piece which is located at the block's end, so in that case it is enough just to resize the block without memmove.

rnpnr commented 9 months ago

So I've looked at this a few times and I still can't quite follow the logic. Can you explain with a little bit more detail so that my slow brain can understand it?

vkochan commented 9 months ago

In my understanding the original idea was "when we are deleting a text from the recent (cached) piece which sits on the edge of the block - then we can just shrink the block size without do a memmove". For example we have a text formed from the following block and pieces:

| [piece1][piece2][piece3 ]| -- this is a block with pieces which holds the inserted chunks | [Some ][text in][ block] | -- these are the chunks per each piece

so, for example we are deleting the "block" from the piece3 which are at the end of the block, then we can just re-size piece3 and block sizes.

So, I think that the original logic does not work if for example the "pos" is at the start of the "block" word and the "len" is length till the end of the block. The real use case might be deleting a text from the end.

rnpnr commented 9 months ago

Ok so I'm still not convinced. I've used this for a while and haven't noticed any issues but that could just be because this is only in the cache path.

The main thing confusing me is that the blk->len parameter is used to mean two different things. I can assume this fix is correct because the check above verifies that pos + len does not lie after the end of the block. The next logical step would be to check if blk->len == pos +len. But on the other hand blk->len apparently also means the insertion position. If you follow the history all the way back this check was added when the blk->len member was called blk->pos. Its possible that this check was wrong then as well but that was 7+ years ago.

A minor nit: pos + len is the value stored in the end variable. That variable should be used in both places where pos + len is being used.