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

Implement user input history #133

Merged

Conversation

jmleong666
Copy link

@jmleong666 jmleong666 commented Oct 21, 2020

Closes #132.

New features/changes

  • Implement navigating user input history on the command box. Pressing the up key while on the command box gets the previous input, while pressing the down key gets the next input. If there is no previous or next input, the command box becomes empty.
  • Add assertions to the archive add and remove commands.

Implementations

  • Introduce a new class UserInputHistory to store all user inputs within the session. User inputs are stored in a LinkedList, and its ListIterator is used to navigate between inputs. Whenever the user enters a command, add the input to the userInputHistory of the command box and reset its iterator.

Possible improvements

  • Write user inputs to a storage file so that the user can access inputs from previous sessions.
  • Improve input history navigation (Pressing down after up returns the same input by the ListIterator navigation logic).

Testing

  • Add unit tests for the UserInputHistory class.

Demo
stonksbook_up_down_demo

@jmleong666 jmleong666 self-assigned this Oct 21, 2020
@jmleong666 jmleong666 added this to the v1.3 milestone Oct 21, 2020
Copy link

@AaronnSeah AaronnSeah left a comment

Choose a reason for hiding this comment

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

LGTM

/**
* Handles the Enter button pressed event.
*/
@FXML
private void handleCommandEntered() {
userInputHistory.addToHistory(commandTextField.getText());
try {

Choose a reason for hiding this comment

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

you might want to consider only adding the text if its not blank?

Copy link
Author

Choose a reason for hiding this comment

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

Yeah should've considered that earlier.

@@ -0,0 +1,54 @@
package seedu.address.ui;

Choose a reason for hiding this comment

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

will unit tests be added in future PR?

Copy link
Author

Choose a reason for hiding this comment

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

Will add them in this PR!

@jmleong666 jmleong666 marked this pull request as ready for review October 21, 2020 12:54
@jmleong666 jmleong666 merged commit 7d22927 into AY2021S1-CS2103T-T11-1:master Oct 21, 2020
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.

As a fast typer with low accuracy, I can retrieve my previous command with a single keystroke
2 participants