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

fix: smart selection not working on the first line of code #1508

Merged
merged 4 commits into from
Oct 27, 2023

Conversation

abeatrix
Copy link
Contributor

fix: smart selection not working on the first line of code

This issue was caused by !selection?.start.line in the smart selection function excluding the possibility of selection?.start.line === 0 (the vs code API returns the line number on display - 1, so line 1 in the editor would returns 0).

This commit fixes that by explicitly checking selection.start.line is not null and is not equal to 0 before failing.

Test plan

Try running the Generate Unit Tests command on the first line of the file:

image

After

You should be able to run the command and the expanded range will be selected for you automatically when available:

image

Before

image

@abeatrix abeatrix requested a review from a team October 26, 2023 14:14
Copy link
Contributor

@dominiccooney dominiccooney left a comment

Choose a reason for hiding this comment

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

Looks good but please simplify, suggestion inline.

vscode/src/editor/vscode-editor.ts Outdated Show resolved Hide resolved
abeatrix and others added 2 commits October 27, 2023 12:07
Co-authored-by: Dominic Cooney <dominic.cooney@sourcegraph.com>
@abeatrix abeatrix merged commit da61767 into main Oct 27, 2023
@abeatrix abeatrix deleted the bee/fix-smart branch October 27, 2023 19:49
@abeatrix abeatrix added the chat/commands Chat and Commands label Nov 1, 2023
burmudar pushed a commit that referenced this pull request Nov 21, 2024
Moving agent to [this commit
](38885b4)
who has a parent which is the[ agent
changes](#4073) from me
## Test plan
Nothing to test here
<!-- All pull requests REQUIRE a test plan:
https://sourcegraph.com/docs/dev/background-information/testing_principles

Why does it matter?

These test plans are there to demonstrate that are following industry
standards which are important or critical for our customers.
They might be read by customers or an auditor. There are meant be simple
and easy to read. Simply explain what you did to ensure
your changes are correct!

Here are a non exhaustive list of test plan examples to help you:

- Making changes on a given feature or component:
- "Covered by existing tests" or "CI" for the shortest possible plan if
there is zero ambiguity
  - "Added new tests"
- "Manually tested" (if non trivial, share some output, logs, or
screenshot)
- Updating docs:
  - "previewed locally"
  - share a screenshot if you want to be thorough
- Updating deps, that would typically fail immediately in CI if
incorrect
  - "CI"
  - "locally tested"
-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chat/commands Chat and Commands
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants