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

New I/O mechanisms print "extra" blank lines on \r #182

Closed
bitprophet opened this issue Aug 19, 2011 · 4 comments
Closed

New I/O mechanisms print "extra" blank lines on \r #182

bitprophet opened this issue Aug 19, 2011 · 4 comments
Labels
Milestone

Comments

@bitprophet
Copy link
Member

Description

When rewriting the I/O loop I had it print the line prefix after any line ending, meaning \n OR \r. I believe I did this because even during testing I ran into cases where a \r was present without a \n, though I'm not sure. It may have simply been pre-emptive on the assumption that somewhere, some program might be using \r as a newline terminator, or someone might be cat-ing such a file.

However, because at least some applications -- such as colorized ls output -- appear to use \r as, well, a carriage return without a newline, we end up with spurious blank, prefixed lines. This looks pretty bizarre, if not downright ugly.

Now, given that the captured text shouldn't have this problem (since the only issue here is that we're printing an extra prefix) it may not be a problem other than cosmetically, but even so, investigate whether a carriage return should be considered a valid line terminator on its own for our prefix-printing purposes. I'm thinking it shouldn't be.


Originally submitted by Jeff Forcier (bitprophet) on 2010-06-21 at 10:32pm EDT

Relations


Closed as Worksforme on 2010-07-13 at 08:56pm EDT

@ghost ghost assigned bitprophet Aug 19, 2011
@bitprophet
Copy link
Member Author

Jeff Forcier (bitprophet) posted:


I can't seem to replicate this now, and upon reflection I'm not sure why exactly the described setup would even be something we could defend against -- if it's creating an actual newline, doesn't that imply that a \n did exist too? (Or was it that since we printed something -- the prefix -- after the \r, it was unable to redraw the line correctly, giving the appearance of a "real" newline? no real idea here)

Closing for now, if I see it again in my travels I'll reopen.


on 2010-07-13 at 08:53pm EDT

@bitprophet
Copy link
Member Author

This is definitely a problem: the example I am seeing while testing #19 (currently using ls / on a number of systems) is that, at the very least, ls does use combo \r\n as line separators (even in situations where no colorization is occurring, FWIW.) This results in extra "blank lines", both in bytewise (with a prefix) and the being-implemented linewise output (as a blank line, no prefix, currently.)

Only obvious solutions are to drop the prefixes or to implement a lookahead functionality. Former is not viable in general use; latter doesn't mesh too well with bytewise output, might be more viable with linewise, but either requires some loop/state juggling.

@bitprophet
Copy link
Member Author

N.B. at least in the bytewise output, the problem is definitely a carriage-return one; when combined with an average-length line prefix and terminal-window-sized output (as is ls's default), this happens (confirmed by screwing with the line prefix and newline-detection code):

  • Initial prefix prints, taking up maybe 16 characters
  • Then ~70 characters of ls output prints on the same line
  • An 80-char-wide terminal window wraps this now 86-char line so that the last 6 characters are on their own line
  • A carriage return (\r) prints, moving the "cursor" to the beginning of the "current" line -- the one with the wrapped/overflow 6 characters.
  • Our newline detection prints another 16-character prefix, which overwrites those 6 characters and then some, again on this 2nd line of terminal output
  • A newline (\n) prints, moving the "cursor" down to another new line (now the 3rd in our terminal window)
  • Newline detection kicks in again, printing another prefix, leaving us ready for the next line of remote stdout/stderr. Rinse, repeat.

This is why both I and others have seen "missing" text (wrapped text is overwritten) and why it seems to appear as spurious newlines.

If corrected, we'll still see semi-ugly line wrapping in this scenario, but at least the reason for it would become obvious and the missing text uncovered.


With an empty-string line prefix, neither problem occurs, because there's no wrapping.

Related thoughts:

  • If we were able to hand-wrap stdout to account for the extra characters in our line prefix, the carriage-return-initiated prefix wouldn't matter, as it would "overwrite" the previous prefix on the same line.
    • However, doing so could be extremely complex and error-prone, if not outright incorrect for some forms of output. Definitely not worth it.
  • I wonder how Capistrano, etc deals with this, since it too has line prefixes. May look into it quickly to sate curiosity.
    • Duh, there's a few reasons they do not have this issue:
      • Cap >=2.1 effectively has pty=False set, so e.g. ls won't print widely by default, and presumably some other programs follow suit
      • More importantly, they always buffer linewise (no interactivity) which allows for simpler prefix handling.
    • Thus, they end up with roughly what we'd like to have, namely correct wrapping.

Whether we can handle this correctly in bytewise output is still unclear, but I think it's possible.

@bitprophet
Copy link
Member Author

Was actually easier than expected; simply consolidate prefix printing to occur before byte-printing (in the place that currently tests whether to print the initial prefix) and examine the tail end of the capture buffer and the current byte. If the buffer ends with a newline character and the current byte is not a newline character, print a prefix before printing the byte.

This results in correct-looking output, with one caveat -- there are still "extra" newlines after each server's chunk of output. However this is because e.g. ls output ends with a trailing \n (actually \r\n, but in this case the \r shouldn't really matter). Capistrano still seems to avoid running into this but it's not clear how (trying to figure it out reminded me why I globbed onto Fab in the first place...)


At this point, then, the newer linewise output in #19 and this fixed bytewise output looks pretty much identical. There are not any actual extra lines compared to the existing bytewise output; it's just that the existing output has the additional line prefixes.

Small consolation though, given that it causes what look like "breaks" between servers -- at least with the final incorrect prefix, scanning the LHS of lines gives a sense of continuity.

Only way to truly fix this probably is a form of lookahead/buffering where we don't print a byte until we've obtained the following byte to figure out if we're at the end, and then to omit printing any final trailing \r\n or \n.

Not 100% sure that's worth it, esp. given it would technically hide actual remote output which might be desired in some situations. There's also the fact that my testing has been limited to ls, using things like git log looks much more natural.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant