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

add [history] spec #2040

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

programmer4229
Copy link

@programmer4229 programmer4229 commented Jul 26, 2023

fixes issue #2039

@withfig-bot
Copy link
Collaborator

CLA Assistant Lite bot:
Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.


I have read the CLA Document and I hereby sign the CLA


GGONZALEZ180 seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You can retrigger this bot by commenting recheck in this Pull Request

@withfig-bot
Copy link
Collaborator

withfig-bot commented Jul 26, 2023

Overview

src/history.ts:

Info:

@withfig-bot
Copy link
Collaborator

Hello @programmer4229,
thank you very much for creating a Pull Request!
Here is a small checklist to get this PR merged as quickly as possible:

  • Do all subcommands / options which take arguments include the args property (args: {})?
  • Are all options modular? E.g. -a -u -x instead of -aux
  • Have all other checks passed?

Please add a 👍 as a reaction to this comment to show that you read this.

@grant0417
Copy link
Member

Hey @programmer4229 can you sign the CLA?

@grant0417 grant0417 added the needs-cla The user needs to sign the CLA before merge label Aug 7, 2023
Comment on lines +4 to +19
subcommands: [
{
name: "0",
description: "Shows full command history",
},
{
name: "|",
description: "Divider to run other commands on history",
subcommands: [
{
name: "grep",
description: "Search history",
},
],
},
],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should not be subcommands. 0 is an argument. (You can pass a number of history items you wish to show.)

The pipe and "grep" should be removed entirely since these are not syntax of the history command, but other shell constructs and commands.

@mschrage mschrage added the needs-change The user needs to make some change before merge label Sep 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-change The user needs to make some change before merge needs-cla The user needs to sign the CLA before merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants