Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CLI mode always involves at least two rows #2196

Closed
dankamongmen opened this issue Sep 21, 2021 · 51 comments · Fixed by #2375
Closed

CLI mode always involves at least two rows #2196

dankamongmen opened this issue Sep 21, 2021 · 51 comments · Fixed by #2375
Assignees
Labels
bug Something isn't working
Milestone

Comments

@dankamongmen
Copy link
Owner

Run e.g. grid or any other CLI-style program that has no text output. There will be a blank line before the new prompt. Running e.g. true or other such tools results in no such blank line, and we oughtn't either.

@dankamongmen dankamongmen added the bug Something isn't working label Sep 21, 2021
@dankamongmen dankamongmen added this to the 3.0.0 milestone Sep 21, 2021
@dankamongmen dankamongmen self-assigned this Sep 21, 2021
@dankamongmen
Copy link
Owner Author

it looks like we're not emitting newlines or anything, but simply tacking a move onto the end that takes us too far. it's the very last thing we're emitting. we know we're on 2 at the beginning -- why do we go to 3? hrmm.

@dankamongmen
Copy link
Owner Author

ok, cup is 1-indexed, and tput handles that for us. so we're doing a correct move. hrmm.

@dankamongmen
Copy link
Owner Author

in xterm i just noticed that sgb-full launched from the top emits no final newline, but if we scroll, one shows up at the bottom. probably the same deal?

@dankamongmen
Copy link
Owner Author

yeah. sgr-full seems a good test. it gives us an extra blank line if it scrolls, and does not otherwise. it never should.

@dankamongmen
Copy link
Owner Author

ncneofetch, interestingly, does not seem to suffer from this problem

@dankamongmen
Copy link
Owner Author

i'm wondering if this is related to ncneofetch scrolling what seems one line short when we're close to the bottom?

@dankamongmen
Copy link
Owner Author

[schwarzgerat](130) $ ./cli 
notcurses 2.4.8 on Kitty 0.23.1 (Linux 5.14.15nlb)
64 rows (22px) 132 cols (11px) 1408x1452 rgb+256 colors
gcc-11.2.0 (LE)
terminfo 6.2.20210905 zlib 1.2.11 GPM n/a
avformat 58.76.100 avutil 56.70.100 swscale 5.9.100 avcodec 58.134.100

0 failed renders, 0 failed rasters, 0 refreshes, 0 input errors
RGB emits:elides: def 0:0 fg 0:0 bg 0:0
Cell emits:elides: 0:0 (0.00%) 0.00% 0.00% 0.00%
Bitmap emits:elides: 0:0 (0.00%) 0B (0.00%) SuM: 0 (0.00%)
[schwarzgerat](0) $ 

this is the absolute minimum notcurses cli:

int main(void){                                                                                                                     
  struct notcurses_options opts = {                                                                                                 
    .flags = NCOPTION_NO_ALTERNATE_SCREEN |                                                                                         
             NCOPTION_PRESERVE_CURSOR |                                                                                             
             NCOPTION_NO_CLEAR_BITMAPS |                                                                                            
             NCOPTION_DRAIN_INPUT,                                                                                                  
  };                                                                                                                                
  struct notcurses* nc = notcurses_init(&opts, NULL);                                                                               
  if(nc == NULL){                                                                                                                   
    return EXIT_FAILURE;                                                                                                            
  }                                                                                                                                 
  notcurses_stop(nc);                                                                                                               
  return EXIT_SUCCESS;                                                                                                              
}

@dankamongmen
Copy link
Owner Author

sans banners, we output (from line 2):

^[[3;1H^[[39;49m^[(B^[[m^[]104^G^[[?25l^[[#P^[[?1002;1006l^[[#Q^[[39;49m^[(B^[[m^[]104^G^[[?1l^[[?12h^[[?25h

@dankamongmen
Copy link
Owner Author

different lines are the exact same output except for that first move. ahhh, it looks like there is a linefeed at the end of our output:

[schwarzgerat](1) $ xxd < f
00000000: 1b5b 3634 3b31 481b 5b33 393b 3439 6d1b  .[64;1H.[39;49m.
00000010: 2842 1b5b 6d1b 5d31 3034 071b 5b3f 3235  (B.[m.]104..[?25
00000020: 6c1b 5b23 501b 5b3f 3130 3032 3b31 3030  l.[#P.[?1002;100
00000030: 366c 1b5b 2351 1b5b 3339 3b34 396d 1b28  6l.[#Q.[39;49m.(
00000040: 421b 5b6d 1b5d 3130 3407 1b5b 3f31 6c1b  B.[m.]104..[?1l.
00000050: 5b3f 3132 681b 5b3f 3235 680a            [?12h.[?25h.
[schwarzgerat](0) $ 

and exit(1) at the end of notcurses_stop_minimal() eliminates it.

@dankamongmen
Copy link
Owner Author

the following seems to remedy the problem, but i'm sure there's some case i'm not considering that prompted this dubious-looking wart:

     if((nc->flags & NCOPTION_PRESERVE_CURSOR) || !get_escape(&nc->tcache, ESCAPE_SMCUP)){
       int targy = nc->rstate.logendy;
       fbuf_reset(&nc->rstate.f);
-      if(++targy >= nc->lfdimy){
+      /*if(++targy >= nc->lfdimy){
         fbuf_putc(&nc->rstate.f, '\n');
         --targy;
-      }
+      }*/

@dankamongmen
Copy link
Owner Author

yeah this has us consuming the emoji line with our closing border in notcurses-info. redirecting stderr to a file puts our prompt on the emoji line. what's different about this case? hrmmm, we draw the logo at the end, do we not? nah, same thing happens with xterm.

maybe we just want a newline in notcurses-info? is that it?

@dankamongmen
Copy link
Owner Author

hah i think that might be it lol

@dankamongmen
Copy link
Owner Author

and to make that work, we do need to bring the cursor out at the end...yes, this is the problem. we need an explicit newline at the end of our output to bump logendy down.

we've absolutely gotta junk that ++targ in shutdown. it's bad.

@dankamongmen
Copy link
Owner Author

i think this only happens if you inhibit banners...?

@dankamongmen
Copy link
Owner Author

int main(void){                                                                                                                     
  struct notcurses_options opts = {                                                                                                 
    .flags = NCOPTION_NO_ALTERNATE_SCREEN |                                                                                         
             NCOPTION_PRESERVE_CURSOR |                                                                                             
             NCOPTION_NO_CLEAR_BITMAPS |                                                                                            
             NCOPTION_DRAIN_INPUT,                                                                                                  
  };                                                                                                                                
  struct notcurses* nc = notcurses_init(&opts, NULL);                                                                               
  if(nc == NULL){                                                                                                                   
    return EXIT_FAILURE;                                                                                                            
  }                                                                                                                                 
  notcurses_stop(nc);                                                                                                               
  return EXIT_SUCCESS;                                                                                                              
}

nope, the program above has banners, and prints an empty line.

@dankamongmen
Copy link
Owner Author

00000140: 7675 7469 6c20 3537 2e38 2e31 3030 2073  vutil 57.8.100 s
00000150: 7773 6361 6c65 2036 2e31 2e31 3030 2061  wscale 6.1.100 a
00000160: 7663 6f64 6563 2035 392e 3132 2e31 3030  vcodec 59.12.100
00000170: 0a1b 5b33 383b 353b 3230 336d 1b28 421b  ..[38;5;203m.(B.
00000180: 5b6d 1b5b 2351 1b5b 3339 3b34 396d 1b28  [m.[#Q.[39;49m.(
00000190: 421b 5b6d 1b5d 3130 3407 1b5b 3f31 6c1b  B.[m.]104..[?1l.
000001a0: 5b3f 3132 681b 5b3f 3235 680a            [?12h.[?25h.

why this LF at the end?

@dankamongmen
Copy link
Owner Author

targy is 63 in this situation, while lfdimy is...0!?

@dankamongmen
Copy link
Owner Author

ok yeah it looks like this is just due to lfdimy never being set because notcurses_render() was never called. that's at least the case for this trivial PoC.

@dankamongmen
Copy link
Owner Author

i might have a fix.

@dankamongmen
Copy link
Owner Author

ok, let's stop flailing around and be a bit rigorous.

  • alternate screen: we always want to have the cursor where we left when we return to the regular screen. this ought be managed by the terminal, so long as all our cursor movements take place within the alternate screen. if we leave and reenter the alternate screen, just put the cursor back where it was upon reentry (does entering the alternate screen always clear the screen? i think it does...yes, it does. do we need refresh upon reentry, then? what do we do with our render context during a temporary departure to the regular screen?)

  • NOTCURSES_OPTION_PRESERVE_CURSOR: now that i think about it, this is only relevant to startup, not shutdown. it means that at startup we ought keep the cursor where it is, as opposed to placing it at the origin. this is true independently of whether we use the alternate screen (ought it be? ought we not default to preserving the cursor when not using the alternate screen?).

so the only case we really need handle is when we've not placed the cursor at the end of our generated output, i.e. when we write something, and then move the cursor backwards in the output. and in this case, we just want to go back to wherever the furthest place we generated output is, right?

dankamongmen added a commit that referenced this issue Nov 23, 2021
@dankamongmen
Copy link
Owner Author

so if that's all correct, we just need to put it at logendy/logendx, right? and those ought be updated whenever we emit something absolutely greater than the current values? except following a scroll operation, in which case we ought reset logendx to 0? isn't that what we were originally doing, though?

@dankamongmen
Copy link
Owner Author

so if that's all correct, we just need to put it at logendy/logendx, right? and those ought be updated whenever we emit something absolutely greater than the current values? except following a scroll operation, in which case we ought reset logendx to 0? isn't that what we were originally doing, though?

i think the problem is that this got mixed up with "last place we wrote", as necessary for goto_location(). yeah, i think we need to continue calculating logendy/logendx as we are, and when they're updated, update the new variables if appropriate. this probably needs be tied tightly in with scrolling, though, since we would want to reset in that case (but not generically on a x=0).

@dankamongmen
Copy link
Owner Author

no... x and y in rstate are the current cursor position. logendy/logendx are for placing of the cursor following each rasterization (presumably in the absence of a visible enabled cursor, which would otherwise override). ok we need trace this entire datapath.

dankamongmen added a commit that referenced this issue Nov 24, 2021
@dankamongmen
Copy link
Owner Author

so if that's all correct, we just need to put it at logendy/logendx, right? and those ought be updated whenever we emit something absolutely greater than the current values? except following a scroll operation, in which case we ought reset logendx to 0? isn't that what we were originally doing, though?

it's a bit more complex than this. we don't want to put the cursor at the last place we wrote; we want to put the cursor one past the last place we wrote (more precisely, immediately past the last glyph we wrote; it might have been more than one column). this is probably best said as: we want the cursor wherever the furthest place our cursor ever was (this accounts for scrolling etc., which ought already have taken place for e.g. a glyph emitted in the lower-right corner).

this would be further complicated by the fact that this works differently in the alternative and regular screens, except that logdimx/logdimy are only relevant for the regular screen, so we'll just always compute that way.

so logendy/logendx get updated in goto_location(), but also get updated in the rastercore, perhaps even in term_putc(). actually, maybe they don't get updated in goto_location() at all.

@dankamongmen
Copy link
Owner Author

ok, things are now everywhere improved over what we had, and close to perfect. problems remaining:

  • ncneofetch: cursor is left on the bottom row, far on the right, when run from the top
  • cli: cursor ends up on bottom row, on the right
  • ncneofetch: when run close to bottom, but not on bottom line, we don't scroll things up enough, and print the info plane partially over the display plane

every other test i've got is now perfect.

@dankamongmen
Copy link
Owner Author

got the first ncneofetch problem resolved.

@dankamongmen
Copy link
Owner Author

got the cli problem resolved.

@dankamongmen
Copy link
Owner Author

so the remaining ncneofetch problem is almost certainly just with it...if we aren't on the bottom, or exactly 17 lines back from the bottom, we don't scroll the logo up enough , and it is partially hidden at the bottom of the screen. in all of these cases, we don't then scroll it up enough for the infoplane, resulting in the infoplane being atop the logo:

2021-11-24-145748_1453x1410_scrot
2021-11-24-145752_1453x1410_scrot

@dankamongmen
Copy link
Owner Author

hrmm so yes, since i changed scroll_down() to only scroll child planes when on the bottom, the logo no longer goes up as expected. ugh, such a tricky problem! hrmm.

@dankamongmen
Copy link
Owner Author

hrmm. this case was already broken, so i think i'm going to aim to merge this, and then we'll figure it out.

we still need to do this btw:

then at render time, we whip up a O(N) array of bools, N==rows, and zero them out a boolean, initialized to 0. whenever we hit a newline, treat it as a nil glyph, but then skip the rest of the line -- solve nothing from further on the line in paint(). furthermore, on the standard plane only, if on the bottom line, and we encounter a newline, set the bool to 1. this ought solve #1590 in and of itself, and without a possibly expensive scrub (only for left-aligned text, though =). finally, in rasterization, if the array is 1 for the given row, we need effect a newline. what would this mean? hrmmm, given the paint() changes from above, i think it all comes down to....if there was a newline on the bottom row, we scroll one. so yeah, we only need O(1) state, not O(N) as i'd thought. ok!

@dankamongmen
Copy link
Owner Author

the one thing i don't like about this is use of \n throughout. that's gonna be CRLF on windows right?

ok, i looked it over. there is no "newline" in ascii/unicode to which \n would natively encode. there is only LF and CR. this is only going to be meaningful when using stdio, where \n will be translated FROM 0xd0xa to 0xa when reading and 0xa TO 0xd0xa when writing on Windows in text mode. it never encodes to two bytes in a string literal, for example.

@dankamongmen
Copy link
Owner Author

i've now got paint() stopping for a plane upon hitting a newline.

i didn't do the bool though, and i've checked, and ncneofetch for instance still gets a newline all the way to raster. how is this happening? and all we're doing with saw_linefeed is setting logendx to 0, not effecting a scroll proper. i guess that's getting handled via the actual scrolling mechanism? but how is it getting to raster?

@dankamongmen
Copy link
Owner Author

i'm going to go ahead and merge this, as it definitely improves things, but i'm a bit perturbed by my failure to understand this render-raster path. it'll need further study, but i've been working on this for three days. bring it home to mama.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant