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

Console: Fixed issues involving Console History not being capped at specified limit, regarding Issue #10598 #11162

Conversation

ShehanAT
Copy link
Contributor

What it does

The history of commands typed in the Debug Console in the Debug view(access in the left sidebar of the browser version) was not adhering to the limit specified in the limit variable in the file: theia/packages/console/src/browser/console-history.ts.

The main issue was that the newly sliced array(this.values.slice(index)) was not being assigned to the original array.

Therefore, I replaced that line with the following: this.values = this.values.slice(index); in order to resolve this issue.

Fixes #10598

How to test

  1. Navigate to the file: theia/packages/console/src/browser/console-history.ts
  2. Change the limit variable to a lower number such as 3
  3. Build the browser version via yarn build browser then navigate to /examples/browser and run yarn start
  4. Navigate to http://localhost:3000 and open the Debug tab in the left sidebar
  5. Enter many commands(at least over the number specified for the limit variable) in the debug console then press the up arrow to validate that the messages saved conforms to the number assigned to the limit variable(Ex: limit = 3 should cap the maximum number of saved commands up to 3)

Review checklist

Reminder for reviewers

…trimmed to maximum, Issue was fixed by assigning trimmed array to original array; Signed-off-by: Shehan Atukorala<shehanatuk@gmail.com>
…trimmed to maximum limit; this commit addresses changing the limit var back to 50; Signed-off-by: Shehan Atukorala<shehanatuk@gmail.com>
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution 👍

In order to accept your changes please be sure to sure to sign the eclipse contributor agreement (eca) with the same email as your authorship.

CHANGELOG.md Outdated Show resolved Hide resolved
examples/browser/webpack.config.js Outdated Show resolved Hide resolved
@ShehanAT
Copy link
Contributor Author

ShehanAT commented May 17, 2022

Thank you for your contribution 👍

In order to accept your changes please be sure to sure to sign the eclipse contributor agreement (eca) with the same email as your authorship.

I have signed the Eclipse Contributor Agreement now. Let me know if I missed anything.

Copy link
Contributor

@JonasHelming JonasHelming left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution!
I will @vince-fugnitto have a final look and then we can merge this one!

@vince-fugnitto vince-fugnitto added the console issues related to the console label May 17, 2022
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

Thank you for your first contribution @ShehanAT 👍

@vince-fugnitto vince-fugnitto merged commit 7d1f724 into eclipse-theia:master May 17, 2022
@vince-fugnitto vince-fugnitto added this to the 1.26.0 milestone May 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
console issues related to the console
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Console History is not trimmed to maximum
3 participants