Re: [dev] [st] [PATCH 3/3] Remove 'slide' variable in tresize.

[Silvan Jegen]

Heyhey

On Sat, Apr 11, 2015 at 05:30:16PM +0000, noname wrote:
> ---
>  st.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/st.c b/st.c
> index 51bd40c..ce2646e 100644

This patch series does not change the logic and results in slightly
fewer lines of code so I like it. Also

Tested-by:s.je...@gmail.com

Combining these patches into a single patch with a more extensive
commit log message describing the changes would have been easier to
review. The patch would have looked similar to this.

diff --git a/st.c b/st.c
index b2bcfe9..45519a3 100644
--- a/st.c
+++ b/st.c
@@ -2769,7 +2769,6 @@ tresize(int col, int row) {
        int i;
        int minrow = MIN(row, term.row);
        int mincol = MIN(col, term.col);
-       int slide = term.c.y - row + 1;
        bool *bp;
        TCursor c;
 
@@ -2779,20 +2778,18 @@ tresize(int col, int row) {
                return;
        }
 
-       /* free unneeded rows */
-       i = 0;
-       if(slide > 0) {
-               /*
-                * slide screen to keep cursor where we expect it -
-                * tscrollup would work here, but we can optimize to
-                * memmove because we're freeing the earlier lines
-                */
-               for(/* i = 0 */; i < slide; i++) {
-                       free(term.line[i]);
-                       free(term.alt[i]);
-               }
-               memmove(term.line, term.line + slide, row * sizeof(Line));
-               memmove(term.alt, term.alt + slide, row * sizeof(Line));
+       /*
+        * slide screen to keep cursor where we expect it -
+        * tscrollup would work here, but we can optimize to
+        * memmove because we're freeing the earlier lines
+        */
+       for(i = 0; i <= term.c.y - row; i++) {
+               free(term.line[i]);
+               free(term.alt[i]);
+       }
+       if(i > 0) {
+               memmove(term.line, term.line + i, row * sizeof(Line));
+               memmove(term.alt, term.alt + i, row * sizeof(Line));
        }
        for(i += row; i < term.row; i++) {
                free(term.line[i]);

better, no?

Thanks for the patch(es)!


Cheers,

Silvan