Skip to content

Conversation

Nathaniel-King-Navarrete
Copy link
Contributor

If the scrollback is less than a minimum amount, then set it to that minimum amount.

Changes

During the instantiate process, set the scollback to a minimum amount. This will globally change the terminal.integrated.scrollback value. If the user has a prefered scollback larger than the minimum, then it will not be update.

How to test this PR

Two test cases:

  1. Scollback value is defined as less than minimum
    1. terminal.integrated.scrollback within settings.json will be updated to 10000.
  2. Scrollback value is defined as equal to minimum or more than minimum.
    1. terminal.integrated.scrollback within settings.json will remain unchanged.

Checklist

  • have tested my change
  • [] have created one or more test cases
  • [] updated relevant documentation
  • Remove any/all console.logs I added
  • have added myself to the contributors' list in CONTRIBUTING.md

sebjulliand and others added 3 commits June 17, 2025 15:05
If the scrollback is less than a minimum amount, then set it to that minimum amount.
@sebjulliand
Copy link
Member

Hi @Nathaniel-King-Navarrete ,
why should Code for IBM i change that global setting?

@Nathaniel-King-Navarrete
Copy link
Contributor Author

Hi @sebjulliand ,

I was using this line as reference point as a possible implementation.

vscode.workspace.getConfiguration().update(`terminal.integrated.sendKeybindingsToShell`, true, true);

Are you thinking that the global is unnecessary for the scrollback?

@worksofliam
Copy link
Member

@Nathaniel-King-Navarrete I think Seb was getting at is that the extension shouldn't really be altering global VS Code configuration for our sake. I agree with him in this instance.

@Nathaniel-King-Navarrete
Copy link
Contributor Author

Sounds good! I will make those changes, when I get the chance.

@worksofliam worksofliam added the stale Inactive label Aug 12, 2025
Updated the terminal.integrated.scrollback to a minimum value.
@Nathaniel-King-Navarrete
Copy link
Contributor Author

Sorry for the delay on this. I think this is what we are looking for.

@worksofliam worksofliam requested review from worksofliam, Copilot and sebjulliand and removed request for Copilot August 12, 2025 14:14
Copy link
Member

@worksofliam worksofliam left a comment

Choose a reason for hiding this comment

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

@sebjulliand what is the consensus here?

@sebjulliand
Copy link
Member

@worksofliam @Nathaniel-King-Navarrete my initial point still stands, which is Code for i should not automatically change the settings (regardless of their scope), at least not without asking the user about it first.
Changing it without warning the user is a bad practice, in my opinion.

Should this setting be changed to fix an issue specific to Code for i, then a modal dialog should be displayed to:

  • explain why the scrollback should be at least xxx
  • display a button to update the setting
  • let the user discard the dialog if they doesn't want that value to be changed

For now, updating it silently is not a good thing.

@Nathaniel-King-Navarrete
Copy link
Contributor Author

@sebjulliand Sounds good. I will make the changes as requested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Inactive
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants