Re: [Patch] Re: Impossibility of modifiable invisible fields

[Thomas Dickey]

On Fri, Aug 31, 2018 at 09:25:51AM +0200, Leon Winter wrote:
> Hi Thomas,
> 
> it is with humble regret I have to admit my failure regarding this issue as I
> have in fact caused this problem when trying to fix it in the first place.
> We discussed this topic in April:
> http://lists.gnu.org/archive/html/bug-ncurses/2018-04/msg00006.html
> 
> I opted for (2) which caused this problem based on a mistaken assumption about
> other terminal programs. Infact !O_PUBLIC should behave like (1) only not
> displaying the change which was a lot easier than expected to achieve.
> My initial fix (for behaviour 1) broke the input just for the benefit of a
> cursor moving "too far".
> I would pledge to revert my initial change and instead just holding back
> rendering for !O_PUBLIC while still computing all the changes (like scrolling)
> which would be in fact the suggested approach (1).
> See attached patch. I would have liked best to give you a patch that looks
> pretty and adheres to the code convention but I there seems to be mix of tabs
> and spaces and I could not decide what is right or wrong :D

I (almost-always) use an indent program (exceptions where you see INDENT-OFF).
 
> > Actually the manual page is vague.  It says:
> > 
> >        O_ACTIVE
> >             The  field  is  visited during processing.  If this option is 
> > off,
> >             the field will not be reachable by navigation keys.  Please 
> > notice
> >             that an invisible field appears to be inactive also.
> > ...
> >        O_PUBLIC
> >             The field contents are displayed as data is entered.
> > ...
> >        O_VISIBLE
> >             The  field  is  displayed.   If this option is off, display of 
> > the
> >             field is suppressed.
> > 
> > I think the best approach is to first clarify in the documentation
> > what the library actually does (and add test-cases which use both
> > features):
> > 
> > a) O_SELECTABLE is an example from the test-program demo_forms.c
> >    (not actually part of the library), but as noted, it combines
> >    O_ACTIVE and O_VISIBLE.
> > 
> > b) O_PUBLIC isn't tested or demonstrated.
> > 
> > With those documented, we can discuss changes starting from that.
> 
> I also looked into that:
> !O_VISIBLE fields are also not selectable.
> Before my "bad fix", !O_PUBLIC fields were able to receive input but displayed
> the cursor incorrectly during input (because scrolling was not done).
> After my "bad fix", !O_PUBLIC fields had a fixed cursor on the first position
> and only allowed for input of one character regardless of the actual dimension
> of field.
> With the attached patch, !O_PUBLIC cursor movement will be correct like 
> O_PUBLIC
> but the content will be not be displayed. This is what the man page suggests
> that *should* happen.

thanks (will study...)
 
> Regards,
> Leon

> diff --git a/form/frm_driver.c b/form/frm_driver.c
> index b07ca7c..e41bbbd 100644
> --- a/form/frm_driver.c
> +++ b/form/frm_driver.c
> @@ -841,17 +841,14 @@ _nc_Position_Form_Cursor(FORM *form)
>    field = form->current;
>    formwin = Get_Form_Window(form);
>  
> -  col = Field_Has_Option(field, O_PUBLIC) ? form->curcol : form->begincol;
> -  row = Field_Has_Option(field, O_PUBLIC) ? form->currow : form->toprow;
> -
> -  wmove(form->w, row, col);
> +  wmove(form->w, form->currow, form->curcol);
>    if (Has_Invisible_Parts(field))
>      {
>        /* in this case fieldwin isn't derived from formwin, so we have
>           to move the cursor in formwin by hand... */
>        wmove(formwin,
> -         field->frow + row - form->toprow,
> -         field->fcol + col - form->begincol);
> +         field->frow + form->currow - form->toprow,
> +         field->fcol + form->curcol - form->begincol);
>        wcursyncup(formwin);
>      }
>    else
> @@ -888,7 +885,7 @@ _nc_Refresh_Current_Field(FORM *form)
>    field = form->current;
>    formwin = Get_Form_Window(form);
>  
> -  if (Field_Has_Option(field, O_PUBLIC))
> +  const bool is_public = Field_Has_Option(field, O_PUBLIC);
>      {
>        if (Is_Scroll_Field(field))
>       {
> @@ -905,6 +902,7 @@ _nc_Refresh_Current_Field(FORM *form)
>                   form->begincol = form->curcol - field->cols
>                     + (move_after_insert ? 1 : 0);
>               }
> +              if (is_public)
>             copywin(form->w,
>                     formwin,
>                     0,
> @@ -965,7 +963,7 @@ _nc_Refresh_Current_Field(FORM *form)
>                 first_modified_row = form->toprow;
>                 first_unmodified_row = first_modified_row + field->rows;
>               }
> -           if (first_unmodified_row != first_modified_row)
> +           if (first_unmodified_row != first_modified_row && is_public)
>               copywin(form->w,
>                       formwin,
>                       first_modified_row,
> @@ -976,14 +974,14 @@ _nc_Refresh_Current_Field(FORM *form)
>                       field->cols + field->fcol - 1,
>                       0);
>           }
> -       wsyncup(formwin);
> +       if (is_public) wsyncup(formwin);
>       }
>        else
>       {
>         /* if the field-window is simply a derived window, i.e. contains no
>          * invisible parts, the whole thing is trivial
>          */
> -       wsyncup(form->w);
> +       if (is_public) wsyncup(form->w);
>       }
>      }
>    untouchwin(form->w);

> _______________________________________________
> Bug-ncurses mailing list
> address@hidden
> https://lists.gnu.org/mailman/listinfo/bug-ncurses


-- 
Thomas E. Dickey <address@hidden>
https://invisible-island.net
ftp://ftp.invisible-island.net

signature.asc
Description: Digital signature