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

Make the 'history' and 'r' commands builtins #76

Merged
merged 5 commits into from
Jul 16, 2020

Conversation

JohnoKing
Copy link

This pull request makes history and r commands builtins that aren't limited to interactive shells. All ksh commands can now be used without unalias -a removing them (unalias -a in .kshrc no longer removes history and r). With this change no more preset aliases exist, so the preset alias tables can be safely removed. The alias tree is now initialized in the same way as the function tree:

shp->fun_tree = dtopen(&_Nvdisc,Dtoset);

The sh_inittree code for comparison:
base_treep = treep = dtopen(&_Nvdisc,Dtoset);

With this change no more preset aliases exist, so the preset alias
tables can be safely removed. All ksh commands can now be used without
'unalias -a' removing them, even in interactive shells. Additionally,
the history and r commands are no longer limited to being used in
interactive shells.

src/cmd/ksh93/bltins/hist.c:
- Implement the history and r commands as builtins. Also guarantee
  lflag is set to one by avoiding 'lflag++'.

src/cmd/ksh93/Makefile,
src/cmd/ksh93/Mamfile,
src/cmd/ksh93/sh/main.c,
src/cmd/ksh93/sh/init.c,
src/cmd/ksh93/data/aliases.c:
- Remove the table of predefined aliases because the last few have
  been removed. During init the alias tree is now initialized the
  same way as the function tree, rather than with a dummy shell table.

src/cmd/ksh93/bltins/typeset.c:
- Remove the now bugfix for unsetting predefined aliases because it
  is now a no-op. Aliases are no longer able to have the NV_NOFREE
  attribute.

src/cmd/ksh93/tests/alias.sh:
- Remove the regression test for unsetting predefined aliases since
  those no longer exist.

src/cmd/ksh93/sh.1:
- Remove the list of preset aliases since those no longer exist.
- Document history and r as builtins instead of preset aliases.
@McDutchie
Copy link

Are there actually scripts that use these?

@JohnoKing
Copy link
Author

JohnoKing commented Jul 16, 2020

Are there actually scripts that use these?

I'm not sure about r, but regarding history there is at least one person that was using the (Bash) history command in a script: https://askubuntu.com/questions/546556/how-can-i-use-history-command-in-a-bash-script
In any case this change was made so unalias -a in the kshrc file doesn't remove the r and history commands from interactive shells, since that wasn't addressed in commit 17f81eb.

@McDutchie
Copy link

True. My assumption was that no existing .kshrc is going to run unalias -a if it wants to keep those aliases. Since these were the only two default aliases that actually made sense as such, I figured my approach fixed things with the minimum change necessary.

OTOH, your approach makes everything consistent and simplifies the code, and that is good. And your way of initialising an empty alias table is clearly better.

I have a couple of tweaks (particularly sh_opthist[] documentation; e.g. now r --man output is weird) but it's probably easiest if I just push those myself.

JohnoKing and others added 4 commits July 16, 2020 09:07
This is just to avoid hard-to-read diffs when comparing across
multiple commits.

src/cmd/ksh93/sh/init.c:
- Rename sh_inittree() back to inittree().
- nv_init(): Move alias_tree init to same line as before.
@McDutchie McDutchie merged commit 03224ae into ksh93:master Jul 16, 2020
@JohnoKing JohnoKing deleted the remove-predefined-aliases branch July 16, 2020 18:02
@hyenias
Copy link

hyenias commented Jul 16, 2020

@JohnoKing @McDutchie:
Help me clarify as to what will happen when my .kshrc issues unalias 2d fc r. I believe you have already removed the 2d functionality. In particular, a have issues with any 1-2 character commands (I do not like stuff happening when a keyboard mishap occurs). As history commands are not something I run that often, simply typing out history or the related hist builtin is easy. However, the fc and r commands fall into my category of must remove. Perhaps instead of issuing unalias I will be able to use builtin -d <name>. Correct?

@JohnoKing
Copy link
Author

builtin -d will work on all of the commands that were previously aliases except times (since that one is a special builtin). fc and r can be removed with builtin -d fc r.

@hyenias
Copy link

hyenias commented Jul 16, 2020

Excellent. Thank you!

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