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

Emacs delete-next-character #51

Merged
merged 1 commit into from
Apr 29, 2022
Merged

Conversation

bsdimp
Copy link
Contributor

@bsdimp bsdimp commented Apr 28, 2022

The historic gnu bc behavior was to have ^D be delete next character if
there were any characters in the input, or end of file if there
weren't. This matches what emacs users expect for editing the command
line. Implement this by assuming end of file if the input is empty, and
delete forward character if it isn't.

Signed-off-by: Warner Losh [email protected]

The historic gnu bc behavior was to have ^D be delete next character if
there were any characters in the input, or end of file if there
weren't. This matches what emacs users expect for editing the command
line. Implement this by assuming end of file if the input is empty, and
delete forward character if it isn't.

Signed-off-by: Warner Losh <[email protected]>
@bsdimp
Copy link
Contributor Author

bsdimp commented Apr 28, 2022

I've been using this change in my tree for a while now since FreeBSD merged the Howard bc. If there's a way to write a test case for it, please let me know.

To test it out, it's simple: 123^B^D^M should leave you in bc and produce the answer 12. Current behavior is to exit after printing 123.

@gavinhoward
Copy link
Owner

Thank you for this submission. I have tested it, and it works as advertised. It does require a tweak to the test suite to get all of the tests to pass.

However, there are two hangups.

First, if I do merge this, I'm not sure it warrants another release by itself; it might be better to wait until some other change comes along.

Second, this does change the behavior of bc and would require a major version bump. This is also the biggest reason for wanting at least one other change; this is a small change that requires a big version bump, and I'd want something big.

But the fact that it changes behavior also gives me pause. I don't know how many people depend on the ^D behavior for end-of-file, even when there's input, and I hesitate to make this change without knowing the numbers.

However, I would be satisfied with just a survey of FreeBSD users, which should be the biggest portion of my userbase. (Not many Linux distros have my bc, and history is not available on Windows.) That is, I would be satisfied if the survey showed that the vast majority of the FreeBSD users who respond either don't care or agree with the change.

I would also be satisfied with the blessing of my regular FreeBSD contact, Stefan Esser, who I believe has a good handle on what changes would be okay to make.

If you can give me either one of those two things, I'll merge. If Stefan even gives a blessing for a release, I'll do that too.

@bsdimp
Copy link
Contributor Author

bsdimp commented Apr 29, 2022

Since it's only in the command history editing, I'm having trouble understanding how it would affect tests or other input since that code path appears to be disabled when we aren't reading from a tty.

And even if it didn't, ^D on Unix is only a tty signal of EOF. It's not used at the end of files to signify EOF, which is done by zero read.

FreeBSD used gnu bc since 1998. It has the behavior I've implemented for the past at least 24 years, so it wouldn't be unreasonable to assume that it's what FreeBSD users that care one way or the other would want. Once you've started to edit the text on the line, you don't want to terminate the program and get an answer for the partially edited line.

I'd argue that this is a mere bug fix to make things more compatible with bc and isn't such a gross behavior change as to warrant a major release bump, but that's just my sensibilities.

I'm OK waiting on a release. I'll also chat with Stefan as well about this point.

@gavinhoward
Copy link
Owner

I have history tests, but they are only run when you run make test_history because they can be...flaky. No worries; I'll make the change to fix them. In fact, I might make a change to test this new behavior.

You make a compelling and convincing case that this is merely a bug fix. Stefan seems to agree that it is and that many old Unix users would expect this behavior change.

In fact, if old Unix users expect the change, then the fact that no one has said anything is probably a good indicator that no one has been using it, which means I can pull the rug out without making anybody mad.

So I'm more than satisfied. I'll make the change, and I'll make it in a bugfix release.

@gavinhoward gavinhoward merged commit f8fa6fe into gavinhoward:master Apr 29, 2022
@gavinhoward
Copy link
Owner

I do have a question: should I remove the EOF behavior entirely?

@bsdimp
Copy link
Contributor Author

bsdimp commented Apr 29, 2022

I do have a question: should I remove the EOF behavior entirely?

No. The behavior I implemented matches gnu bc's behavior.

@stesser
Copy link
Contributor

stesser commented Apr 29, 2022

I had sent a reply to Gavin a few hours ago, and I do see that the pull request has already been merged.

Just a comment regarding compatibility with the previous bc version in FreeBSD:

While GNU bc has the requested behavior, the previous bc in FreeBSD up to 12.x did not:

$ bc -v
bc (BSD bc) 1.1-FreeBSD
$ bc
12345^B^B^B <- cursor on 3
^D <- bell sound, nothing deleted

1245 <- result after pressing DEL

In the FreeBSD bc (which had been obtained from OpenBSD) ^D did only signal EOF if the input line was empty, but it did not delete to the right.

GNU bc actually has the line editing behavior created by this patch, and thus it is further step towards GNU bc compatibility. (And IIRC, GNU bc had been in the FreeBSD base system, a few decades ago ...)

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

Successfully merging this pull request may close these issues.

3 participants