#56381 closed enhancement (fixed)
It would be nice if shell mode saved command history immediately
Reported by: | ylluminarious (George Plymale II) | Owned by: | ylluminarious (George Plymale II) |
---|---|---|---|
Priority: | Normal | Milestone: | MacPorts 2.6.0 |
Component: | base | Version: | 2.4.3 |
Keywords: | Cc: | raimue (Rainer Müller) | |
Port: |
Description
Hi,
I'm new to Macports so I hope that this ticket is not a duplicate and that it meets the Ticket Guidelines. Anyway, as I was using Macports' shell mode, I noticed history is only saved after the main command loop exits. This approach has a major drawback (which I've often been a victim of while using bash
).
If the port
process is abruptly killed, then that session's history is gone. This can happen by pressing ^C
twice, which can quite easily happen as an accident. There are also a myriad of other dangers which could abruptly kill a port
shell session before its time.
It would be nice if each command got written to the history file right after it was entered. This is how zsh
does it and I've always been fond of it, since you always know that your history is saved. I looked in src/port/port.tcl to see if there was a setting to enable such behavior, but I see no such setting there. Specifically, I looked in the process_command_file
procedure, but found nothing relating to this.
Attachments (2)
Change History (16)
comment:1 follow-up: 2 Changed 7 years ago by pmetzger (Perry E. Metzger)
comment:2 Changed 7 years ago by ylluminarious (George Plymale II)
Replying to pmetzger:
I suspect no one else will be as interested in working on this as you might be, but patches to add an optional mode to save immediately, if cleanly implemented, would almost certainly be accepted. Are you interested in working on it?
Hi, sorry for the somewhat delayed reply. Yes, I am interested in working on this. I probably need to brush up on my Tcl, but aside from that I don't see a problem. I'll try and submit a patch in the next few weeks or so.
comment:3 follow-ups: 4 5 Changed 7 years ago by raimue (Rainer Müller)
Cc: | raimue added |
---|
Hello, and welcome to MacPorts! Yes, you already found the right place with process_command_file
in the port client.
I guess the solution would be to call rl_history write
before executing the command. Then it would always be saved, even if the command itself is interrupted. Could even be done in get_next_cmdline
where the line is added to the readline history in memory.
One drawback is that this would always write the full history file instead of just appending a single line. It looks like readline (actually libedit) has a append_history
function we are not yet wrapping in rl_history that should be used for appends.
Let us know if you get stuck or need assistance when working on this!
comment:4 Changed 7 years ago by ylluminarious (George Plymale II)
Replying to raimue:
Hello, and welcome to MacPorts! Yes, you already found the right place with
process_command_file
in the port client.I guess the solution would be to call
rl_history write
before executing the command. Then it would always be saved, even if the command itself is interrupted. Could even be done inget_next_cmdline
where the line is added to the readline history in memory.One drawback is that this would always write the full history file instead of just appending a single line. It looks like readline (actually libedit) has a
append_history
function we are not yet wrapping in rl_history that should be used for appends.Let us know if you get stuck or need assistance when working on this!
Hi! Thanks so much for your advice. Those are certainly some good pointers and I'm sure it will help me out as I'm implementing this.
Honestly, I've begun to use Macports out of some disillusionment with Homebrew. That project is more and more taking away features and estranging certain kinds of users with certain use-cases. It seems that this is largely due to the maintainers' open and flagrant disregard for users. I am glad that it seems that the Macports project is more welcoming to improvements.
comment:5 Changed 6 years ago by ylluminarious (George Plymale II)
Hi again folks,
I've been working on this today and I've run into a bit of a wall. I think I've successfully wrapped the append_history
function and I've also got some code which uses it in port.tcl
. Yet, for some reason the use_readline
variable is always false when I run a custom build of port
. I'm using this custom build as per the instructions in the guide about running multiple installations of Macports.
It seems that use_readline
is false because of the second clause in its definition:
set use_readline [expr {$isstdin && [readline init "port"]}]
.
That is, the [readline init "port"]
command seems to be failing for some reason. I have no idea why that would be even after examining its definition here. Moreover, my code does not touch anything in the ReadlineCmd
function.
I've attached a patch which contains my code. I currently have some debugging puts
statements in it because of the above-mentioned bug. So that's why they're there, if you're wondering.
Some environment information:
- I'm on macOS 10.13.4
- The custom Macports build is on commit
a6aa79a2
(which is v2.4.4) - I've got a real Macports installation under
/opt/local
- The custom Macports build has no ports installed and I've not run
selfupdate
on it
I would appreciate any help that could be offered.
Changed 6 years ago by ylluminarious (George Plymale II)
Attachment: | macports_append_history.patch added |
---|
First draft of a patch to fix this issue
comment:6 follow-up: 7 Changed 6 years ago by raimue (Rainer Müller)
You need to use ./configure --enable-readline
as it is disabled by default. HAVE_LIBREADLINE
would not be defined on your installation.
comment:7 Changed 6 years ago by ylluminarious (George Plymale II)
Replying to raimue:
You need to use
./configure --enable-readline
as it is disabled by default.HAVE_LIBREADLINE
would not be defined on your installation.
Thanks again for the advice! You are right, I thought that option was on by default but didn't double-check.
However, I am now running into a different issue with this error when I try to recompile my changes:
Undefined symbols for architecture x86_64: "_append_history", referenced from: _RLHistoryCmd in readline.o ld: symbol(s) not found for architecture x86_64
After grepping through Apple's libedit, it seems that actually there is no append_history
function defined anywhere. It seems to only be in GNU readline... so this does put a bit of a wrench in things.
comment:8 Changed 6 years ago by ylluminarious (George Plymale II)
Just FYI: I've dug a little more into solving this problem, but haven't found much. So I posted a question on StackOverflow about it. Hopefully we can get some pointers on what to do here.
comment:9 follow-up: 10 Changed 6 years ago by raimue (Rainer Müller)
Indeed, if libedit does not implement append_history
that is really unfortunate. Looks like it was only recently added to NetBSD, which would be where Apple borrowed their implementation, but it has also been patched since the initial import. It is unlikely that this will find its way into macOS, definitely not into older releases. Filing a rdar might help.
Not sure how we want to continue with this, then. Do we want to roll our own code to manage the history file? I have doubts that this is worth importing the whole libedit source to our vendor libraries...
comment:10 Changed 6 years ago by ylluminarious (George Plymale II)
Replying to raimue:
Indeed, if libedit does not implement
append_history
that is really unfortunate. Looks like it was only recently added to NetBSD, which would be where Apple borrowed their implementation, but it has also been patched since the initial import. It is unlikely that this will find its way into macOS, definitely not into older releases. Filing a rdar might help.
Yes, you're right. While fishing around for ideas, I found the original source of libedit and it seems that Apple has not made many changes to their distribution of it over the years. I doubt they would help even after filing a rdar... I'm very doubtful that they'd backport old releases of macOS which are no longer supported.
Not sure how we want to continue with this, then. Do we want to roll our own code to manage the history file? I have doubts that this is worth importing the whole libedit source to our vendor libraries...
Yes, importing the whole of libedit is not worth this one feature. I'd agree that to do so would be overkill. All of this has led me to the conclusion that we need to roll our own solution or drop the whole issue entirely. I prefer the former, so I've come up with a rather simplistic solution which may actually suffice.
I poked and prodded around Apple's sources, NetBSD's sources, and finally the GNU info manual for their History library and I finally wrote a few lines which actually seem to do the job. The key was that I found the current_history()
function which yields the most recent history item. Its return value can be used to access the raw string via the line
field. I.e., current_history()->line
After finding this function, I wrote a couple of lines with just fopen
and fprintf
and it seems to work. The code is very basic; there's no error-handling on I/O errors and there's no encoding/decoding of the history string. However, it seems that such en/de-coding is not necessary. This surprised me at first, until I realized that macOS's bash
is also using Readline (I think) and it has no such encoding wherein the space character is translated to \040
and so forth.
Readline seems to be able to pull history items out of the list even if they are using raw whitespace, without any hitch whatsoever. I tried also to send lines with lots of different characters such as braces, semicolons, ampersands, and all kinds of stuff and Readline did not so much as hiccup. I'm surprised and I'm hoping that this code is sufficient to solve this issue. Please let me know what you think and if you have any improvements in mind.
Thanks.
Changed 6 years ago by ylluminarious (George Plymale II)
Attachment: | 2nd_macports_append_history.patch added |
---|
comment:11 Changed 6 years ago by ylluminarious (George Plymale II)
Friendly ping... any thoughts on these changes?
comment:12 Changed 6 years ago by ylluminarious (George Plymale II)
Owner: | set to ylluminarious |
---|---|
Resolution: | → fixed |
Status: | new → closed |
comment:13 Changed 6 years ago by neverpanic (Clemens Lang)
Milestone: | → MacPorts Future |
---|
comment:14 Changed 5 years ago by jmroot (Joshua Root)
Milestone: | MacPorts Future → MacPorts 2.6.0 |
---|
I suspect no one else will be as interested in working on this as you might be, but patches to add an optional mode to save immediately, if cleanly implemented, would almost certainly be accepted. Are you interested in working on it?