willghatch / racket-rash

The Reckless Racket Shell
http://rash-lang.org
Other
551 stars 31 forks source link

ls causes issues with history-delete #86

Open Gavinok opened 3 years ago

Gavinok commented 3 years ago

when I run the ls command in rash I get the following error

history-delete: index out of history range, -7 - 6
  context...:
   /usr/share/racket/pkgs/readline-lib/readline/rktrl.rkt:126:0: history-delete
   /usr/share/racket/pkgs/readline-lib/readline/pread.rkt:67:0: add-to-history
   /usr/share/racket/pkgs/readline-lib/readline/pread.rkt:137:0: do-multiline-chunk
   /usr/share/racket/pkgs/readline-lib/readline/pread.rkt:168:11
willghatch commented 3 years ago

Sorry for the delayed reply. Busy...

This looks like, as with so many other issues on the tracker, an issue with the readline library (or rather Racket's FFI wrapper for it). I doubt this is specific to ls. Does this happen with every command you run?

At any rate, the real solution, like most other issues in the tracker, is that I need to write a better line editor. Some day I'll get to that...

On Sun, May 02, 2021 at 05:34:34PM -0700, Gavinok wrote:

when I run the ls command in rash I get the following error

history-delete: index out of history range, -7 - 6
 context...:
  /usr/share/racket/pkgs/readline-lib/readline/rktrl.rkt:126:0: history-delete
  /usr/share/racket/pkgs/readline-lib/readline/pread.rkt:67:0: add-to-history
  /usr/share/racket/pkgs/readline-lib/readline/pread.rkt:137:0: do-multiline-chunk
  /usr/share/racket/pkgs/readline-lib/readline/pread.rkt:168:11

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/willghatch/racket-rash/issues/86

xlambein commented 3 years ago

I have the same error. It happens every time I run ls, but it doesn't seem to be an issue with other commands. I've tried out cat, ps & git.

willghatch commented 3 years ago

Huh. It doesn't make any sense to me that it would be specific to ls. At any rate, I haven't been able to reproduce it. I don't really have time to dig into this right now, but I'll ask some more questions to hopefully narrow things down. No hurry to answer, but:

glyh commented 3 years ago

I met the same problem...

willghatch commented 3 years ago

Can you answer any of the above questions? I would like to fix this, but until I have more information I'm in the dark.

On Fri, Aug 06, 2021 at 12:36:18AM -0700, Lyhokia wrote:

I met the same problem...

-- You are receiving this because you commented. Reply to this email directly or view it on GitHub: https://github.com/willghatch/racket-rash/issues/86#issuecomment-894067398

glyh commented 3 years ago
  1. Arch linux
  2. I don't have rashrc, nor do rash generate one for me.
  3. Version
    User-specific for installation "8.2":
    Package  Checksum                Source
    rash     c40c5adfedf632bc1fd...  catalog...it?path=rash
  4. Y
  5. I don't know what programs are you talking about, but I don't use DrRacket as my main IDE. I've opened it for 2~3 times.
  6. no, a plain install with nothing, just make the problem appear.
wothie commented 3 years ago

Hello, I encountered the same problem, although on a different "command". I hope that this is still the right thread to post, as the fundamental issue seems to be the same. Specifically, for me the error happens on the line (+ 3 4).

What's curious is that I couldn't reproduce it with anything else (only tried for a few minutes though). Other additions work, whether they have 1, 2, or more summands, (* 3 4) is fine, and ls didn't cause issues for me either.

The output:

[b@b-80xl ~]$ racket -l rash/repl

You can use the `man` command to get documentation about programs.  Try running `man man`.
(To turn these hints off, run (current-repl-display-startup-hints? #f) in a rashrc file.)
15:28 /home/b/
> (+ 3 4)
Result 1:
7
15:28 /home/b/
history-delete: index out of history range, -0 - -1
  context...:
   /usr/share/racket/pkgs/readline-lib/readline/rktrl.rkt:252:0: history-delete
   /usr/share/racket/pkgs/readline-lib/readline/pread.rkt:68:0: add-to-history
   /usr/share/racket/pkgs/readline-lib/readline/pread.rkt:138:0: do-multiline-chunk
   /usr/share/racket/pkgs/readline-lib/readline/pread.rkt:169:11

I also find interesting that the crash happens after the result and the next prompt are already printed, yet without any further input after (+ 3 4)<enter>. Also the problem seems to have disappeared after I removed ~/.config/racket/racket-prefs.rktd

To answer the above questions:

This might not be the most useful contribution, but maybe it helps a little bit ^^ I'll keep you updated, if I can reproduce the error again, but it might take a little, as I'm supposed to be studying right now ;)

john9631 commented 2 years ago

Ran into the same issue.

My solution was to add a replacement ls file to my ~/.local/bin path which has is before /usr/bin and /bin in my shells path.

So, create a new file ~/.local/bin/ls

Edit the file to:

#!/bin/bash

/usr/bin/ls -G $@

Make it executable with chmod +x ~/.local/bin/ls

and restart hash-repl. Problem gone.

A variation if you don't want to change the paths is to rename your existing ls with mv /usr/bin/ls /usr/bin/zzls and use /usr/bin/zzls -G $@ in the new ls file which can be stored in /usr/bin in place of the original ls. Don't forget to make it executable with sudo chmod +x /usr/bin/ls

bugsbugsbux commented 2 years ago

same problem here, @john9631's solution does not work for me.

i've also had @wothie's problem with (+ 3 4) yesterday, but i cannot reproduce that today (which is strange as i neither updated nor rebooted; rebooting after discovering this did not fix the ls problem and addition is still somehow fixed...)

in termux on android i dont see any problems

Rinzwind commented 2 years ago

I can reproduce the problem using a Vagrant machine set up as follows (with bento/centos-7.9):

vagrant init bento/centos-7.9 &&
vagrant up &&
vagrant ssh --command "
  sudo yum install --assumeyes epel-release &&
  sudo yum install --assumeyes --enablerepo=epel-testing racket racket-doc &&
  sudo raco pkg install --scope installation --auto --skip-installed rash &&
  sudo chmod a+x /usr/bin/rash-repl" &&
vagrant snapshot save 'Finished Installation'

Once that’s done, do:

vagrant snapshot restore 'Finished Installation' && vagrant ssh

At the shell prompt, start rash-repl, enter ls -a, press Ctrl-D. This gives:

10:39 /home/vagrant/
> ls -a
.  ..  .bash_logout  .bash_profile  .bashrc  .prlctl_version  .ssh
10:39 /home/vagrant/
> ^Dptr-ref: contract violation
  expected: (and/c cpointer? (not/c (lambda (p) (pointer-equal? p #f))))
  given: #f
  argument position: 1st
  other arguments...:
   #<ctype>
  context...:
   /usr/share/racket/pkgs/readline-lib/readline/rktrl.rkt:105:4: ffi-wrapper:history_get
   /usr/share/racket/pkgs/rash/repl.rkt:187:12: save-readline-history!
   /usr/share/racket/pkgs/rash/repl.rkt:64:0: rash-repl
   /usr/share/racket/pkgs/rash/repl.rkt:129:0: main
   (submod "/usr/share/racket/pkgs/rash/repl.rkt" main): [running body]
   for-loop
   run-module-instance!125

Restart rash-repl, and enter ls -a again. This gives:

10:40 /home/vagrant/
> ls -a
.  ..  .bash_logout  .bash_profile  .bashrc  .prlctl_version  .racket  .ssh
10:40 /home/vagrant/
history-delete: index out of history range, -0 - -1
  context...:
   /usr/share/racket/pkgs/readline-lib/readline/rktrl.rkt:109:4: ffi-wrapper:remove_history
   /usr/share/racket/pkgs/readline-lib/readline/rktrl.rkt:119:0: history-delete
   /usr/share/racket/pkgs/readline-lib/readline/pread.rkt:168:11
willghatch commented 2 years ago

Cool, thanks @Rinzwind !

I've been super busy lately (graduating, moving, starting a job, and I'm expecting my wife to go into labor any day now), so I haven't had any time to work on this stuff.

That said, when I get some free time, I intend to try the relatively new expeditor package for Rash line editing instead of the readline package. It will likely be more robust all around and solve issues like this.

Rinzwind commented 2 years ago

Ok, no problem (and congratulations!). For others having this issue: as a workaround, try applying the following patch to disable saving of the ‘readline’ history.

Patch (as a file: repl.rkt.patch.gz):

--- /usr/share/racket/pkgs/rash/repl.rkt    2022-04-20 11:39:22.000000000 -0700
+++ -   2022-04-20 12:51:56.656459533 -0700
@@ -186,9 +186,7 @@
       (set! save-readline-history!
             (λ ()
               ;; TODO - I should trim this to a maximum length.
-              (define rash-history
-                (for/list ([i (in-range (history-length))])
-                  (history-get i)))
+              (define rash-history '()) ;; PATCHED, see: https://github.com/willghatch/racket-rash/issues/86
               (put-preferences '(rash:repl:readline-input-history)
                                (list rash-history))))))

To apply it on the Vagrant machine setup I gave above:

vagrant snapshot restore 'Finished Installation' &&
vagrant ssh --command "
  sudo yum install --assumeyes patch &&
  gunzip --stdout /vagrant/repl.rkt.patch.gz | sudo patch /usr/share/racket/pkgs/rash/repl.rkt" &&
vagrant ssh
willghatch commented 2 years ago

I could also just add an option to not save/load history in the source (rather than as a patch). Maybe that's the best option for fixing this while using the readline library.

Rinzwind commented 2 years ago

Please do, having an option would be more convenient!

One thing I was wondering about while looking at the code is why save-readline-history! exists as separate from the save-history in ‘pread.rkt’ (based on what winds up in the file .racket/racket-prefs.rktd, that seems to be what is used to save the history for the racket REPL, in which the issue with ffi-wrapper:history_get doesn’t seem to occur).

willghatch commented 2 years ago

Ok, I've just pushed a commit that adds a --readline-persistent-history option that you can set to true or false. Maybe before I go to bed I'll look at what it would take to add expeditor support. I don't think it will be very hard.

willghatch commented 2 years ago

I also made a branch with an experimental repl-expeditor file. So far when it works it works better than with readline (eg. with some highlighting, reasonable auto-indentation, etc), but it has some weird issues. For example, I'm not confident it is any better at not borking the racket-prefs.rkt file.

willghatch commented 2 years ago

One thing I was wondering about while looking at the code is why save-readline-history! exists as separate from the save-history in ‘pread.rkt’ (based on what winds up in the file .racket/racket-prefs.rktd, that seems to be what is used to save the history for the racket REPL, in which the issue with ffi-wrapper:history_get doesn’t seem to occur).

I wrote a different function to save it with a different key, so that rash-repl and regular racket repl history don't get mixed up. I think I've had issues both ways. I'm not sure what the core problem is.

Rinzwind commented 2 years ago

I wrote a different function to save it with a different key, so that rash-repl and regular racket repl history don't get mixed up.

Ah right, I thought I was just missing something about how the storage location for the history is parameterized in the library, but it really is just hardcoded then.

Thanks for adding the option, seems to work fine now for me with that disabled!