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

Show PyPi link in requirements #23288

Merged

Conversation

LouisGobert
Copy link

Show PyPi link in requirements, see #14495.

Apart from the test to verify the register of RequirementsTxtLinkActivator, there are no other tests. This is my first contribution in TypeScript, if you have an example of a test that could be done, I'm interested :)

@LouisGobert
Copy link
Author

@microsoft-github-policy-service agree

Comment on lines 14 to 18
// Regex to allow to find every possible pypi package (base regex from https://peps.python.org/pep-0508/#names)
const regex = /^([A-Z0-9]|[A-Z0-9][A-Z0-9._-]*)($|=| |;|\[)/i
const row = document.lineAt(position.line).text;
const projectName = row.match(regex);
return(projectName) ? new Hover(`https://pypi.org/project/${projectName[1]}/`) : null
Copy link
Member

Choose a reason for hiding this comment

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

You can pull this out into a function by itself that takes in a string and returns hover info. Then you can test that function independently.

@karthiknadig karthiknadig self-assigned this Apr 23, 2024
@karthiknadig karthiknadig added the feature-request Request for new features or functionality label Apr 23, 2024
@karthiknadig karthiknadig added this to the May 2024 milestone Apr 23, 2024
Copy link
Member

@karthiknadig karthiknadig left a comment

Choose a reason for hiding this comment

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

Thanks for adding this feature, and the test cases. This mostly looks good. I have some minor suggestions, overall looks great.

LouisGobert and others added 3 commits April 24, 2024 20:05
Co-authored-by: Karthik Nadig <kanadig@microsoft.com>
Co-authored-by: Karthik Nadig <kanadig@microsoft.com>
Co-authored-by: Karthik Nadig <kanadig@microsoft.com>
LouisGobert and others added 2 commits April 24, 2024 22:27
@LouisGobert
Copy link
Author

@karthiknadig No, I think it's an issue in test:

[18:15:56] Using gulpfile ~/work/vscode-python/vscode-python/path with spaces/gulpfile.js
[18:15:56] Starting 'prePublishNonBundle'...
[18:15:56] Starting 'compile'...
[18:15:56] Starting 'compileCore'...
src/test/activation/requirementsTxtLinkActivator.unit.test.ts(38,37): error TS2345: Argument of type 'string | null' is not assignable to parameter of type 'string'.
  Type 'null' is not assignable to type 'string'.
TypeScript: 1 semantic error
TypeScript: emit succeeded (with errors)
[18:16:11] 'compileCore' errored after 15 s
[18:16:11] Error: TypeScript compilation errors
    at Pumpify.<anonymous> (/home/runner/work/vscode-python/vscode-python/path with spaces/gulpfile.js:38:44)
    at Pumpify.emit (node:events:526:35)
    at Pumpify.emit (node:domain:552:15)
    at finishMaybe (/home/runner/work/vscode-python/vscode-python/path with spaces/node_modules/readable-stream/lib/_stream_writable.js:630:[14](https://github.com/microsoft/vscode-python/actions/runs/8821193135/job/24216944026#step:5:15))
    at afterWrite (/home/runner/work/vscode-python/vscode-python/path with spaces/node_modules/readable-stream/lib/_stream_writable.js:492:3)
    at onwrite (/home/runner/work/vscode-python/vscode-python/path with spaces/node_modules/readable-stream/lib/_stream_writable.js:483:7)
    at WritableState.onwrite (/home/runner/work/vscode-python/vscode-python/path with spaces/node_modules/readable-stream/lib/_stream_writable.js:180:5)
    at Object.onceWrapper (node:events:628:28)
    at Pumpify.emit (node:events:514:28)
    at Pumpify.emit (node:domain:552:[15](https://github.com/microsoft/vscode-python/actions/runs/8821193135/job/24216944026#step:5:16))
[18:16:11] 'compile' errored after 15 s
[18:[16](https://github.com/microsoft/vscode-python/actions/runs/8821193135/job/24216944026#step:5:17):11] 'prePublishNonBundle' errored after 15 s

karthiknadig
karthiknadig previously approved these changes May 6, 2024
anthonykim1
anthonykim1 previously approved these changes May 6, 2024
@anthonykim1
Copy link

anthonykim1 commented May 6, 2024

There's still lint error on src/client/activation/requirementsTxtLinkActivator.ts

@karthiknadig
Copy link
Member

@LouisGobert Sorry for another ping on this. Can you format the following with prettier extension?
image

@LouisGobert LouisGobert dismissed stale reviews from anthonykim1 and karthiknadig via 900a0a5 May 6, 2024 20:13
@LouisGobert
Copy link
Author

@karthiknadig Done:
image

@karthiknadig karthiknadig enabled auto-merge (squash) May 7, 2024 20:49
@karthiknadig karthiknadig merged commit 569ca65 into microsoft:main May 7, 2024
35 checks passed
anthonykim1 pushed a commit to anthonykim1/vscode-python that referenced this pull request May 10, 2024
Show PyPi link in requirements, see microsoft#14495.

Apart from the test to verify the register of
`RequirementsTxtLinkActivator`, there are no other tests. This is my
first contribution in TypeScript, if you have an example of a test that
could be done, I'm interested :)

---------

Co-authored-by: Karthik Nadig <kanadig@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Request for new features or functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants