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.

Sign up for GitHub

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

Jump to bottom

repl: REPL history seems broken in master and v11.2.0 #24385

Closed
vsemozhetbyt opened this issue Nov 15, 2018 · 12 comments
Closed

repl: REPL history seems broken in master and v11.2.0 #24385

vsemozhetbyt opened this issue Nov 15, 2018 · 12 comments
Labels
repl Issues and PRs related to the REPL subsystem.

Comments

@vsemozhetbyt
Copy link
Contributor

vsemozhetbyt commented Nov 15, 2018 •
edited
Loading

  • Version: v11.2.0, v12.0.0-nightly201811153212f77ac6 or 'v12.0.0-v8-canary20181115bc23a47c3b'
  • Platform: Windows 7 x64
  • Subsystem: repl
  • Delete .node_repl_history.
  • Launch node.
  • Type:
> 1
1
  • Look into .node_repl_history, there is 1 there.
  • Type:
> 2
2
  • Look into .node_repl_history, there is only 2.
  • Type:
> 3
3
  • Look into .node_repl_history, there is only 3.
  • Type UP ARROW several times. There is only:
> 3
  • Close node.
  • Look into .node_repl_history, there is only 3 there.

Can anybody reproduce?
@vsemozhetbyt vsemozhetbyt added the repl Issues and PRs related to the REPL subsystem. label Nov 15, 2018
@vsemozhetbyt vsemozhetbyt changed the title repl: REPL history seems broken in master repl: REPL history seems broken in master and v11.2.0 Nov 15, 2018
@vsemozhetbyt
Copy link
Contributor Author

I cannot reproduce with v11.1.0.
I can reproduce with v11.2.0.

@vsemozhetbyt
Copy link
Contributor Author

vsemozhetbyt commented Nov 15, 2018 •
edited
Loading

cc @antsmartian as per possible cause #22153 backported in v11.2.0.

@antsmartian
Copy link
Contributor

@vsemozhetbyt 😢 Yes, thats what caused the regression. My bad.

@antsmartian
Copy link
Contributor

antsmartian commented Nov 16, 2018 •
edited
Loading

@vsemozhetbyt Working on a PR, on the way. Also, I guess we need to tighten our history test cases, as this should have been caught in first place :( :( .

@antsmartian antsmartian mentioned this issue Nov 16, 2018
Closed
4 tasks
Fishrock123 pushed a commit that referenced this issue Nov 19, 2018
@antsmartian @Fishrock123
repl: handle buffered string logic on finish
c11d345 
Looks like `clearBufferedCommand` will be called on almost all flows.
Hence history was broken.

PR-URL: #24389
Fixes: #24385
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
targos pushed a commit that referenced this issue Nov 19, 2018
@antsmartian @targos
repl: handle buffered string logic on finish
682b3d5 
Looks like `clearBufferedCommand` will be called on almost all flows.
Hence history was broken.

PR-URL: #24389
Fixes: #24385
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
rvagg pushed a commit that referenced this issue Nov 28, 2018
@antsmartian @rvagg
repl: handle buffered string logic on finish
bd8be40 
Looks like `clearBufferedCommand` will be called on almost all flows.
Hence history was broken.

PR-URL: #24389
Fixes: #24385
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
@caub
Copy link

caub commented Dec 1, 2018

it's still broken in v11.3.0
peek 2018-12-01 21-52

@vsemozhetbyt
Copy link
Contributor Author

11.3.0 was a security concerning release, so as per the changelog the fix was not backported yet,

@vsemozhetbyt
Copy link
Contributor Author

The fix is on v11.x-staging branch, so this may be fixed in the next 11 release.

This was referenced Dec 4, 2018
Closed
Closed
Trott pushed a commit to Trott/io.js that referenced this issue Dec 15, 2018
@antsmartian @Trott
test: adding history regression test case
0bf7d41 
PR-URL: nodejs#24843
Refs: nodejs#24385
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
BethGriggs pushed a commit that referenced this issue Dec 18, 2018
@antsmartian @BethGriggs
test: adding history regression test case
eb9e6e6 
PR-URL: #24843
Refs: #24385
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
sagitsofan pushed a commit to sagitsofan/node that referenced this issue Dec 24, 2018
@antsmartian @sagitsofan
test: adding history regression test case
6061d24 
PR-URL: nodejs#24843
Refs: nodejs#24385
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
refack pushed a commit to refack/node that referenced this issue Jan 14, 2019
@antsmartian @refack
repl: handle buffered string logic on finish
ce5021e 
Looks like `clearBufferedCommand` will be called on almost all flows.
Hence history was broken.

PR-URL: nodejs#24389
Fixes: nodejs#24385
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
refack pushed a commit to refack/node that referenced this issue Jan 14, 2019
@antsmartian @refack
test: adding history regression test case
7739375 
PR-URL: nodejs#24843
Refs: nodejs#24385
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
BethGriggs pushed a commit that referenced this issue Feb 12, 2019
@antsmartian @BethGriggs
test: adding history regression test case
54d7e82 
PR-URL: #24843
Refs: #24385
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
BethGriggs pushed a commit that referenced this issue Feb 20, 2019
@antsmartian @BethGriggs
test: adding history regression test case
d7a6218 
PR-URL: #24843
Refs: #24385
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
rvagg pushed a commit that referenced this issue Feb 28, 2019
@antsmartian @rvagg
test: adding history regression test case
1348cd5 
PR-URL: #24843
Refs: #24385
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@RichardBronosky
Copy link

I install node via homebrew on macOS. Can someone please explain how I can get this fix?

@caub
Copy link

caub commented Apr 5, 2019

update node

@RichardBronosky
Copy link

I have

$ node -v
v11.13.0

and I am still getting this behavior.

@caub
Copy link

caub commented Apr 6, 2019

Oh, it might be a problem on macOS then, for me on v11.13.0 and linux, the problem doesn't exist anymore (it was fixed since v11.4.0 actually)

@RichardBronosky
Copy link

I was having a near breakdown from stress and didn't notice the difference between v11.3.0 and v11.13.0. I had for former version and the issue before upgrading to the later but didn't notice the number change and thought I was on the latter from the start. I then went out and bought a OneWheel to burn off some stress.

Thanks for the help though!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
repl Issues and PRs related to the REPL subsystem.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@RichardBronosky @antsmartian @caub @vsemozhetbyt