Skip to content

Set sidebar location when no preference #288

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ItWasEnder
Copy link

@ItWasEnder ItWasEnder commented Mar 17, 2025

Description ✏️

In a recent change the sidebar location was being override on extension mount. This PR changes this logic to only set the sidebar location if the user has not set it already.

If the indented change was to force a "default layout" I would suggest creating some setting within the PearAI configuration, or include some flag that overrides this behavior.

Checklist ✅

  • I have added screenshots (if UI changes are present).
  • I have done a self-review of my code.
  • I have manually tested my code (if applicable).

Important

Modify activateExtension in activate.ts to set sidebar location only if not user-defined.

  • Behavior:
    • In activate.ts, activateExtension now sets workbench.sideBar.location to 'left' only if not already set by the user.
  • Misc:
    • Suggests creating a setting or flag for default layout enforcement in PearAI configuration.

This description was created by Ellipsis for c6e95e3. It will automatically update as commits are pushed.

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

❌ Changes requested. Reviewed everything up to c6e95e3 in 1 minute and 22 seconds

More details
  • Looked at 19 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 drafted comments based on config settings.
1. extensions/vscode/src/activation/activate.ts:111
  • Draft comment:
    Consider checking both global and workspace settings. Currently, only globalValue is checked. If a user has set a workspace-specific setting for sidebar location, it might be overlooked.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50%
    The comment makes a valid technical point - workspaceValue could indeed be set independently of globalValue. However, looking at the broader context, this code is trying to set a default sidebar location if the user hasn't explicitly set it anywhere. The third parameter 'true' in the update() call indicates this is explicitly setting it globally. The current approach of only checking globalValue seems intentional - if there's a workspace setting, we probably want to override it globally anyway.
    I could be wrong about the intention - maybe we really do want to preserve workspace-specific settings. The code comment "If the user has not set the sidebar location" is ambiguous about whether it means globally or anywhere.
    The code is clearly setting a global value (note the 'true' parameter), suggesting it intentionally wants to establish a global default regardless of workspace settings. The current implementation aligns with this goal.
    The comment should be deleted. The current implementation appears intentional in only checking globalValue since it's setting a global default.
2. extensions/vscode/src/activation/activate.ts:113
  • Draft comment:
    Minor typographical suggestion: consider adding a comma in the comment, e.g. "If the user has not set the sidebar location, set it" to improve readability.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
    This is an extremely minor stylistic suggestion about punctuation in a comment. It doesn't affect code functionality or even code quality in any meaningful way. According to the rules, we should not make purely informative comments or comments that are obvious/unimportant.
    Perhaps clear and readable comments are important for maintainability, and this suggestion does technically improve readability slightly.
    While readable comments are good, this change is too minor to warrant a PR comment. It's more noise than signal in the review process.
    Delete this comment as it's too minor and doesn't meaningfully improve code quality.
3. extensions/vscode/src/activation/activate.ts:162
  • Draft comment:
    There is an inconsistency in naming: the comment on line 162 refers to 'PearAPP' while the rest of the file and project name use 'PearAI'. Please confirm and correct if necessary.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_nLiP3enspKP7SLYE


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

const sidebar = vscode.workspace.getConfiguration().inspect("workbench.sideBar.location");

// If the user has not set the sidebar location set it
if (!sidebar?.globalValue) {
Copy link

Choose a reason for hiding this comment

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

Consider checking both global and workspace values. Relying solely on globalValue can override a user’s workspace setting. For example, use if (!sidebar?.globalValue && !sidebar?.workspaceValue) to determine if no preference was set.

Suggested change
if (!sidebar?.globalValue) {
if (!sidebar?.globalValue && !sidebar?.workspaceValue) {

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.

1 participant