hist and fc: only run edited commands if changes are saved.

[pghvlaans]

According to The New KornShell, the full editor history editing commands are only meant to execute command lines that have had saved changes:

"The command is the ksh line that you were editing with the vi built-in editor if you omit n, or command n in the history file. When you exit the vi program, ksh displays and executes the command if you have changed it. Version: With some early releases of the 11/16/88 version of ksh, the file was executed even if you had not changed it." (Bolsky & Korn, pg. 116)

There is no other evidence that this functionality was ever part of release ksh93, however. Neither bash nor any existing open source ksh88 derivative behaves in this manner. Nevertheless, some distributions of ksh88 (such as the one shipped with UnixWare 7) did attempt to avoid running unchanged command lines using timestamps. The code in the new implementation is entirely different:

• bltins/hist.c: When the full editor has been called, only execute the line being viewed if the contents have been changed and saved.
• sh.1, data/builtins.c: Update the hist documentation accordingly.
• tests/builtins.sh: Increase ulimit for the hist test to accommodate changes to hist.c.

[pghvlaans]

Just a few quick notes about my testing round this time:

• I no longer have access to a working macOS VM.
• OmniOS and Haiku both experienced complete test failures on dev and my own branch. I'll try to look into this and open an issue.

[pghvlaans]

Got rid of a couple of stray variables from testing and renamed some new variables I wasn't happy about.

[posguy99]

Not documented in the Bolsky (as far as I know) are the ^X prefix commands supported by the emacs line editor. Chief among those is ^X ^E, as you know for bringing the current line into a full screen editor.

ksh, mksh, and bash all execute the line on return from the editor, even if there were no changes.

Since this patch isn't to either editor source file, it affects both?

[pghvlaans]

ksh, mksh, and bash all execute the line on return from the editor, even if there were no changes.

Indeed. Personally, I think the book got it right and that's a bit of an anti-pattern, since the normal result of exiting an editor without saving is "nothing happens."

Since this patch isn't to either editor source file, it affects both?

Yes, that's right.

[posguy99]

I tested it locally, works for me in vi or emacs mode.

[pghvlaans]

Awesome, thanks!

[posguy99]

@McDutchie Would you consider merging this, since it brings ksh in line with the Bolsky?

[McDutchie]

Yes, I'm mulling it over and I've been doing some research.

I've researched the code for all the ksh93 versions in the ast-open-archive repo. And I have found no evidence that emacs ^X^E in ksh has ever acted like Bolsky & Korn describe.

So is ksh wrong, or the book? I lean towards the latter, particularly as all other shells also act like ksh does now.

That doesn't mean the change wouldn't be good to have, but perhaps it should be for the future 93u+m/1.1 release (the current dev branch) and not the 1.0 series.

[posguy99]

Bolsky doesn't mention the emacs variant at all, only the vi variant. Bolsky documents the vi variant, but says that the command line is only executed when the line was changed, while ksh always executes it.

The man page for ksh does document both the emacs variant and the vi variant, but doesn't state how either behaves.

[pghvlaans]

So is ksh wrong, or the book? I lean towards the latter, particularly as all other shells also act like ksh does now.

That seems like a fair conclusion. The 1995 code I looked at when writing the original comment had neither mode doing this.

Since this change would apparently be a matter of personal taste rather than book accuracy, I'll make another commit to put it behind a shell option.

[McDutchie]

I'm reluctant to add a shell option for something this minor. If we go that route we may eventually end up like zsh which has too many shell options. I think it would be better just to make a decision one way or another.

[pghvlaans]

OK, I have a theory about what might've happened: checking for saved changes was a feature of some distributions of ksh88, but was written out of ksh93 altogether, either from the start or too early to get picked up by ksh-history.

In this UnixWare 7 repository, there is source for both ksh and ksh93. In ksh, builtin.c, line 1260:

/* if the file hasn't changed, treat this as a error */
if(*a1!='-' && fstat(fdo,&statb)>=0 && before==statb.st_mtime)
	sh.exitval = 1;

These lines are not present in the ksh93 version of hist.c in that same repository.

The Solaris 2.6 code also has some insight. Their (ksh88) builtin.c has the file check present but enclosed in notdef at line 1427. A code comment reads:

/*
 * Korn says this is the right thing to do, but our customers,
 * the ksh man page, the ksh book, and the POSIX shell spec
 * all disagree with Korn.  None of the docs make any mention
 * of checking for modification of the file.  And besides, the
 * mod check fails if you modify the file *very* quickly.
 */

If the Solaris people of that time are to be believed, the feature was controversial and (surprise, surprise) buggy. Although I do like it (and the code works differently), I'd be happy to drop this if you believe there might be user pushback.

[posguy99]

It seems clear to me now that the current behavior is intentional, since the code was there, and was removed in the re-write. Currently it's the book that has the problem with how it works.

It just looks to me that editing a line and executing a line should be separate acts. If you bring back history into the line editor, you can do whatever you want to it, but never actually execute it, just by moving off of it to another line. With the full editor, SOMETHING is always going to get executed when you leave the editor, there's no way out unless you empty the buffer and save THAT.