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

Command prompt not cleared after each command ran #499

Closed
novadev94 opened this issue Mar 31, 2020 · 52 comments
Closed

Command prompt not cleared after each command ran #499

novadev94 opened this issue Mar 31, 2020 · 52 comments

Comments

@novadev94
Copy link

Environment details (Put x in the checkbox along with the information)

[x] Operating System: macOS Mojave 10.14.6
[x] Desktop Environment: -
[x] Terminal Emulator: Terminal.app, iTerm, Kitty (all 3)
[x] Shell: bash, zsh, fish (all 3, changed via chsh -s)
[x] Custom desktop opener (if applicable): -
[x] Issue exists on nnn master: -

Exact steps to reproduce the issue

  1. Open nnn
  2. ] to go to command prompt
  3. echo 1 and Enter twice
  4. ] to go to command prompt again
  5. Prompt shows text like er' to continue

https://asciinema.org/a/w3S0QPbey7oekD7QD2s73Kxko

@jarun
Copy link
Owner

jarun commented Mar 31, 2020

  • Are you using the readline version?
  • Is the prompt clear?

I see something like the following with zsh and bash (output from the previous command and prompt clean and ready for next input):

>> echo 1
1

'Enter' to continue
>>> 

@jarun
Copy link
Owner

jarun commented Mar 31, 2020

Note: This is from master.

@jarun
Copy link
Owner

jarun commented Mar 31, 2020

I see the asciinema now.

@novadev94
Copy link
Author

novadev94 commented Mar 31, 2020

I built nnn using readline installed from brew

❯ brew info readline
readline: stable 8.0.4 (bottled) [keg-only]
Library for command-line editing
https://tiswww.case.edu/php/chet/readline/rltop.html
/usr/local/Cellar/readline/8.0.4 (48 files, 1.5MB) *
  Poured from bottle on 2020-02-22 at 02:06:28
From: https://github.com/Homebrew/homebrew-core/blob/master/Formula/readline.rb
==> Caveats
readline is keg-only, which means it was not symlinked into /usr/local,
because macOS provides the BSD libedit library, which shadows libreadline.
In order to prevent conflicts when programs look for libreadline we are
defaulting this GNU Readline installation to keg-only.

For compilers to find readline you may need to set:
  set -gx LDFLAGS "-L/usr/local/opt/readline/lib"
  set -gx CPPFLAGS "-I/usr/local/opt/readline/include"

For pkg-config to find readline you may need to set:
  set -gx PKG_CONFIG_PATH "/usr/local/opt/readline/lib/pkgconfig"

==> Analytics
install: 584,614 (30 days), 1,398,146 (90 days), 5,431,859 (365 days)
install-on-request: 59,821 (30 days), 132,377 (90 days), 534,079 (365 days)
build-error: 0 (30 days)
/oss/nnn master
❯ make all
cc -I/usr/local/opt/readline/include -Wall -Wextra -O3  -L/usr/local/opt/readline/lib -o nnn src/nnn.c -lreadline -lncurses

But it also happens with the default readline too.

@novadev94
Copy link
Author

I tried it inside Vim (via nnn.vim), and seems like there's no problem with nnn -p - since the prompt is displayed at the bottom of the screen
image

@jarun
Copy link
Owner

jarun commented Mar 31, 2020

I tried it inside Vim (via nnn.vim), and seems like there's no problem with nnn -p - since the prompt is displayed at the bottom of the screen

This is the built-in prompt (not the readline one). In picker mode this is used. You can compile nnn without libreadline using the make option O_NORL=1.

My readline version is 7.0-3. I am not seeing this issue on kitty (tested latest release available) as well.

@jarun
Copy link
Owner

jarun commented Mar 31, 2020

I compiled with readline 8.0 now. And still I don't see the issue.

Probably the problem is not with libreadline/nnn as they don't handle printing to the terminal and moving the pointer forward to the end of the input.

@KlzXS any ideas?

@novadev94
Copy link
Author

novadev94 commented Mar 31, 2020

I just tried switching to readline 7.0.5 (compile from source) and the problem still persists. It also happens with other terminal emulator and shell too.

Here's a vanilla /bin/bash in Terminal.app
image

@KlzXS
Copy link
Collaborator

KlzXS commented Mar 31, 2020

@KlzXS any ideas?

I can't reproduce this. Is this a mac specific bug?

@jarun
Copy link
Owner

jarun commented Mar 31, 2020

@KlzXS looks like that.

@novadev94 can you try with stock readline? Also, just to double check, you are on master, right?

@vuduyhung94
Copy link

I'm having the same problem: zsh, iTerm, macOS 10.14.6, installed via: brew install nnn --HEAD
image

@novadev94
Copy link
Author

@jarun I did a brew uninstall readline and rebuilt nnn, same problem. Please note that the 'Enter' to continue line isn't shown right after echo 1<enter>, and I need to input another <enter> to exit the prompt.

@0xACE
Copy link
Collaborator

0xACE commented Mar 31, 2020

Instead of hitting enter when 'Enter' to continue appears, what happens if you type osx? Does it appear as 'Enter' to continueosx or does it remain as 'Enter' to continue?

@0xACE
Copy link
Collaborator

0xACE commented Mar 31, 2020

Actually looking at the asciicinema, it looks like we should flush stdout aswell..

@jarun
Copy link
Owner

jarun commented Mar 31, 2020

Can someone confirm if the following patch works?

diff --git a/src/nnn.c b/src/nnn.c
index 8266882..a183637 100644
--- a/src/nnn.c
+++ b/src/nnn.c
@@ -1713,6 +1713,7 @@ static int spawn(char *file, char *arg1, char *arg2, const char *dir, uchar flag
 				while (getchar() != '\n');
 			}
 
+			fflush(stdout);
 			refresh();
 		}
 

@0xACE
Copy link
Collaborator

0xACE commented Mar 31, 2020

I don't think flushing is the solution to the problem, but it should at least fix them not seeing the prompt...

@jarun
Copy link
Owner

jarun commented Mar 31, 2020

Please try the following patch:

diff --git a/src/nnn.c b/src/nnn.c
index 8266882..c806446 100644
--- a/src/nnn.c
+++ b/src/nnn.c
@@ -1711,8 +1711,8 @@ static int spawn(char *file, char *arg1, char *arg2, const char *dir, uchar flag
                        if (flag & F_CONFIRM) {
                                printf("%s", messages[MSG_CONTINUE]);
                                while (getchar() != '\n');
+                               fflush(stdout);
                        }
-
                        refresh();
                }
 

@novadev94
Copy link
Author

@0xACE

  • ] to go to prompt
  • echo 1<enter>
  • osx<enter>
  • prompt closes, nnn shows
  • ] to go to prompt
    => It remains 'Enter' to continue (with the first 4 characters hidden under >>> )
    image

@jarun Putting fflush(stdout) before while and after printf actually works for me. Just want to confirm if this is the correct behavior: I can input anything freely IN the same line as 'Enter' to continue, but those chars are ignored until I press <enter> which returns to nnn?

diff --git a/src/nnn.c b/src/nnn.c
index 942d950..2343784 100644
--- a/src/nnn.c
+++ b/src/nnn.c
@@ -1710,6 +1710,7 @@ static int spawn(char *file, char *arg1, char *arg2, const char *dir, uchar flag
 		if (flag & F_NORMAL) {
 			if (flag & F_CONFIRM) {
 				printf("%s", messages[MSG_CONTINUE]);
+				fflush(stdout);
 				while (getchar() != '\n');
 			}

image

@0xACE
Copy link
Collaborator

0xACE commented Mar 31, 2020

Yes, that is correct. I'd say the place where you placed fflush() is the correct placement.

And yes, anything you type until you hit enter is practically ignored, even though they echo out on the screen...

If you can verify that flush() solved this problem (and I'd vote that flush goes where @novadev94 put it). Then we could have it merged up stream. (The reason I'm asking again, is because it's unclear in your message if flush() solved the problem completely)

Otherwise I have a solution in place for __APPLE__ devices... but I didn't want to push it until we found a better solution.

@jarun jarun closed this as completed in 47ec055 Mar 31, 2020
@jarun
Copy link
Owner

jarun commented Mar 31, 2020

I think he intends to confirm that the fflush() in between works as intended. I would push the change as common for all platforms. Shouldn't harm.

@jarun
Copy link
Owner

jarun commented Mar 31, 2020

@novadev94 and @vuduyhung94 please confirm the patch with and without readline (-p works without readline).

@lawnowner
Copy link
Contributor

fflush doesn't solve the issue before of after getchar loop, with or without readline, in a pty or tty, on my end on a linux. You can reproduce the issue by pressing Enter more than once after typing sleep 1;echo 1 at ] prompt. Adding a newline after the continue message seems to produce a satisfactory effect: "\n'Enter' to continue\n".

@jarun
Copy link
Owner

jarun commented Mar 31, 2020

@lawnowner how about typing additional chars at the prompt before pressing Enter? Does it work in that case too?

@lawnowner
Copy link
Contributor

Anything typed during command execution is buffered and output at the end of execution, before the continue message, as I think it should; normally we don't press keys hysterically while the command is executing. With a newline after the continue message, the next prompt line is clear, and the continue message is displayed before it.

@jarun
Copy link
Owner

jarun commented Mar 31, 2020

normally we don't press keys hysterically while the command is executing.

My case is:

'Enter' to continue
type some garbage text here, then Enter

@novadev94
Copy link
Author

@jarun Latest commit on master works for me, in both nnn and nnn -p -. Thank you!

@0xACE Sorry, I didn't make it clear in my message. I just wanted to confirm the "anything you type until you hit enter is practically ignored" is the correct behavior since I haven't seen it in action xD.

@jarun
Copy link
Owner

jarun commented Mar 31, 2020

@novadev94 looks like @lawnowner has found the actual issue.

jarun added a commit that referenced this issue Mar 31, 2020
@jarun
Copy link
Owner

jarun commented Mar 31, 2020

What's the status line? The message?

@lawnowner
Copy link
Contributor

lawnowner commented Mar 31, 2020

No, the nnn status line showing the file count, size, etc. It corrupts the ] prompt as they are shown on the same location of the screen on a tty. Well, at least on my end.

@novadev94
Copy link
Author

novadev94 commented Mar 31, 2020

@jarun Commit da8b257 has a weird behavior on tty for me (Mac), it still works ok on pty.

  • ] to go to prompt
  • echo 1<enter>
  • 'Enter' to continue NOT printed
    image
  • <enter> to go back to nnn
  • ] to go to prompt again
    image

Putting the fflush(stdout) back in makes it work flawlessly again, and seems like it's the only way to fix it on my machine.

@novadev94
Copy link
Author

Combining both the \n'Enter' to continue\n and fflush(stdout) patches fixes #499 (comment) on my machine too.

@jarun
Copy link
Owner

jarun commented Mar 31, 2020

@lawnowner can you confirm everything works with the suggestion in #499 (comment)?

@lawnowner
Copy link
Contributor

That's what I meant by "fflush doesn't solve the issue..."; both fflush and continue\n clears the next prompt line. The nnn status line still corrupts the ] prompt on a tty however.

jarun added a commit that referenced this issue Mar 31, 2020
@0xACE
Copy link
Collaborator

0xACE commented Mar 31, 2020

The thing with flush was only meant to enforce printing the 'Enter' to continue message. Adding "\n" to the message isn't really the solution, the reason the error is not perceived is because now the prompt is jumping down a line. The "problem" is still there, but now we just can't see it. On linux this solution creates an unnecessary newline, not that I care...

What is going on is that our >>> prompt is not starting on a new line. It's just going back to col=0 and then printing >>>. I have the same problem in mutt but it just doesn't bother me enough to do something about it...

There are many methods for solving this, some of the quick and dirty hacks are:

  1. We make sure >>> goes to a new/fresh line before prompting.
  2. We clear out the current line >>> is about to prompt on.
  3. We clear out the 'Enter' to continue message from the screen before we return to nnn (Does not solve terminals which do not handle "fullscreen")
  4. We make sure __APPLE__ prints a newline after that message. (Does not solve terminals which do not handle "fullscreen")

A proper solution would flush right after the printf message. And then see if "fullscreen" support exists or running on __APPLE__: if it's on __APPLE__ or if there is not "fullscreen" support prompt \n>>>, everything else can keep the >>> prompt. And even this solution is incorrect as the enter prompt is still echoing your characters... But we are building a tiny tool, not a luxury car...

Sorry I've spent too much time on this problem, I must go on with my life. Good night.

@0xACE
Copy link
Collaborator

0xACE commented Mar 31, 2020

in case anyone is dealing with this until my next session: tput el may be relevant see man terminfo clr_eol or clr_bol

@lawnowner
Copy link
Contributor

lawnowner commented Mar 31, 2020

Change the prompt string with \n>>> and remove the \n at the end of the continue message, then the ] prompt line is clear for both pty and tty, and we can have luxurious night all together ;]

@jarun
Copy link
Owner

jarun commented Mar 31, 2020

@lawnowner do we still need the fflush(stdout)?

@lawnowner
Copy link
Contributor

@jarun No.

jarun added a commit that referenced this issue Mar 31, 2020
@jarun
Copy link
Owner

jarun commented Mar 31, 2020

This should be gone for good now.

Note: I made a force push to keep the commit history clean.

@novadev94
Copy link
Author

Thank you guys for all your time!

For the record, as 0xACE did with the great explanation above, fflush is needed for

The thing with flush was only meant to enforce printing the 'Enter' to continue message.

So to be precise, there were actually 2 problems at first

  1. 'Enter' to continue is hidden by the >>> prompt -- This one is annoying, and can be fixed by all the proposed solutions in this discussion. It's merely one of the "symptoms" for a bigger issue explained by 0xACE.

  2. 'Enter' to continue not printed after running a command, and before waiting for an enter -- This one is not a problem, and as long as there's no fflush, it won't be fixed.

After the latest fix (91bd84c), it behaves exactly the same as mentioned here: #499 (comment) -- (1) fixed but not (2).

Just want to point it out since saying fflush not needed isn't really accurate, as it helps to fix the other half of the original issue. That's why I suggested #499 (comment). @lawnowner didn't have (2) problem at first, so of course fflush is not needed for him. But it may not mean it's not needed on my machine, or all Mac machines I have tested on.

@0xACE
Copy link
Collaborator

0xACE commented Apr 1, 2020

There has been some miscommunication:

Problem 1.
fflush() is needed to enforce that the message is printed. Commonly amongst OSs: they may buffer your stdout until they seem necessary to print. You can explicitly tell it to flush, or hope and pray that \n will convince the OS to flush... This is the reason the message did not appear.

Problem 2.
>>> was behaving different for us, the easiest solution for all would be \n>>>. The problem that was perceived was that on OSX hitting enter on the getchar() didn't print a \n as it did on linux, so the next time the >>> prompt spawned this would cause this mishap. So if you ever ensured a \n is printed before >>> you wouldn't have noticed this problem. But then the other guy mentioned >>> being broken in tty, which most likely didn't support "fullscreen", further indicating a overall safe bet is \n>>>

I don't have the time to deal with this right now. I looked at the code and it seemed fine, though there is still one place where >>> is used rather than \n>>>, but I didn't look up the context of that code... As of now the current master looks good enough if you ask me.

@jarun
Copy link
Owner

jarun commented Apr 1, 2020

@0xACE

though there is still one place where >>> is used rather than \n>>>

The one without ``n` is the in-built prompt shown in status bar. We do not switch out of ncurses mode for this.

@novadev94

I'll add the fflush().

jarun added a commit that referenced this issue Apr 1, 2020
@jarun
Copy link
Owner

jarun commented Apr 1, 2020

Done! Whoever talks about this next raises a PR.

@jarun
Copy link
Owner

jarun commented Apr 7, 2020

@novadev94 we need a help from you. None of us is on macOS, Can you please add some compilation steps related to Homebrew for macOS in the Wiki? Something like the Pi or Android steps in this page: https://github.com/jarun/nnn/wiki/Developer-guides#compile-for-pi

Things users would be interested in:

  • Get master and compile on Homebrew
    • tools to install like compiler, automake, readline etc.
  • Get master and compile with make options

Please add examples of the commands. The wiki is publicly editable BTW.

@novadev94
Copy link
Author

@jarun I tried installing from source with a fresh macOS 10.13. And it worked fine with stock readline (libedit on macOS), ncurses and make, no need for installing any external dependencies. Note that it's a fresh machine, so even brew is not installed.

As with "Get master and compile on Homebrew", the --HEAD option is already there in the Brew info (this one is on my machine, not the fresh one)

/oss/nnn master
❯ brew info nnn
nnn: stable 3.0 (bottled), HEAD
Tiny, lightning fast, feature-packed file manager
https://github.com/jarun/nnn
/usr/local/Cellar/nnn/HEAD-341b1cc (10 files, 132KB) *
  Built from source on 2020-04-07 at 23:39:07
From: https://github.com/Homebrew/homebrew-core/blob/master/Formula/nnn.rb
==> Dependencies
Required: readline ✔
==> Options
--HEAD
	Install HEAD version
==> Caveats
Bash completion has been installed to:
  /usr/local/etc/bash_completion.d

zsh completions have been installed to:
  /usr/local/share/zsh/site-functions

fish completions have been installed to:
  /usr/local/share/fish/vendor_completions.d
==> Analytics
install: 846 (30 days), 5,009 (90 days), 16,090 (365 days)
install-on-request: 845 (30 days), 4,999 (90 days), 16,055 (365 days)
build-error: 0 (30 days)

I'm happy to help but I'm not sure what should I write about here :D. Of course it's possible to install a custom version of readline/ncurses and build nnn using them. But for a relatively modern version of macOS, it should work out of the box.

@jarun
Copy link
Owner

jarun commented Apr 8, 2020

What if someone wants to use make options: https://github.com/jarun/nnn/wiki/Developer-guides#make-options

@novadev94
Copy link
Author

@jarun Just tried all the options, only O_PCRE=1 had problems if pcre isn't installed. I updated the wiki to include these

On macOS:

    brew install pcre  # if you're using Homebrew
    sudo port install pcre  # if you're using MacPorts
    make O_PCRE=1 strip

@jarun
Copy link
Owner

jarun commented Apr 8, 2020

Thank you!

@novadev94
Copy link
Author

O_STATIC also doesn't work on macOS.

And also on macOS gcc man page: http://mirror.informatimago.com/next/developer.apple.com/documentation/DeveloperTools/gcc-4.0.1/gcc/Link-Options.html

-static
On systems that support dynamic linking, this prevents linking with the shared libraries. On other systems, this option has no effect.
This option will not work on Mac OS X unless all libraries (including libgcc.a) have also been compiled with -static. Since neither a static version of libSystem.dylib nor crt0.o are provided, this option is not useful to most people.

I left a note in the wiki page.

@jarun
Copy link
Owner

jarun commented Apr 8, 2020

Thanks again!

0xACE pushed a commit to 0xACE/nnn that referenced this issue May 5, 2020
0xACE pushed a commit to 0xACE/nnn that referenced this issue May 5, 2020
@lock lock bot locked and limited conversation to collaborators May 20, 2020
@jarun
Copy link
Owner

jarun commented May 11, 2021

Guys, can you please confirm this issue is not re-introduced with commit 29baa7c?

Don't want to break the next release.

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

No branches or pull requests

6 participants