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

Open the specific version directory and manifest.json (or other specified file) when version is downloaded #47

Merged
merged 13 commits into from
Jun 10, 2024

Conversation

chrstinalin
Copy link
Contributor

@chrstinalin chrstinalin commented May 27, 2024

Closes #36, closes #32 and implements #31 and #43 on the Assay side.

Changes

  • Whenever ‘Review New Addon Version’ in VS Code or ‘Open in VS Code’ is used, the VS Code window now opens to the add-on version directory directly in a new instance of VS Code and defaults to its associated manifest.json, OR
  • Given a link of form vscode://mozilla.assay/review/<guid>/<version>?path=<path>#L<line(s)>, the file located at <path> will be opened in stead of manifest.json, focused on <line(s)> (ex. #L25, #L25-30...).
  • User can select a line as they usually do to make a comment, then copy a link to that selection, OR copy the link via an existing comment.
    image
    image
    Video Link

Note: Since Assay is currently running in a development host window, the new windows it opens are regular instances of VS Code -- i.e, ones that do not have the Assay extension. Thus, its functionality is not carried over with external windows, and the manifest.json (or other file) will not open. This would not be an issue if Assay is a properly installed extension, as it is toggled to start on launch of a new VS Code instance. For testing purposes, you can set forceNewWindow to false on line 21 of openFromurl.ts, OR bundle and install the extension using vsce.

  • Instead of creating a new workspace or stacking on top of the existing one every time a new version is opened (which prompts the user to save the workspace every time they want to exit), the directory is opened directly in the explorer via vscode.openFolder.

  • The commenting/review system is only launched if the user is inside the root folder. That way, Assay does not get in the way of the user’s other VS Code projects. Only the Assay commands menu launches (from where the user can still download by URL, enter their API key/secret, etc.)

@chrstinalin chrstinalin requested a review from dotproto May 27, 2024 19:39
@chrstinalin chrstinalin changed the title opens manifest.json when version is downloaded or opened Open the specific version directory and manifest.json file when version is downloaded or opened May 27, 2024
@chrstinalin chrstinalin changed the title Open the specific version directory and manifest.json file when version is downloaded or opened Open the specific version directory and manifest.json file when version is downloaded May 27, 2024
@codecov-commenter
Copy link

codecov-commenter commented May 28, 2024

Codecov Report

Attention: Patch coverage is 81.50289% with 32 lines in your changes missing coverage. Please review.

Project coverage is 90.18%. Comparing base (dcbe5f9) to head (e53d4ca).
Report is 18 commits behind head on main.

Files Patch % Lines
src/utils/getThreadLocation.ts 53.84% 12 Missing ⚠️
src/commands/openFromUrl.ts 82.22% 7 Missing and 1 partial ⚠️
src/commands/getAddon.ts 0.00% 7 Missing ⚠️
src/extension.ts 92.00% 4 Missing ⚠️
src/utils/commentManager.ts 96.42% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #47      +/-   ##
==========================================
- Coverage   90.67%   90.18%   -0.49%     
==========================================
  Files          23       24       +1     
  Lines        1608     1702      +94     
  Branches      137      137              
==========================================
+ Hits         1458     1535      +77     
- Misses        149      165      +16     
- Partials        1        2       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@chrstinalin chrstinalin changed the base branch from comment-api to main May 28, 2024 18:29
@chrstinalin chrstinalin changed the title Open the specific version directory and manifest.json file when version is downloaded Open the specific version directory and manifest.json (or other specified file) when version is downloaded May 29, 2024
@chrstinalin chrstinalin marked this pull request as draft May 29, 2024 14:58
@chrstinalin chrstinalin marked this pull request as ready for review May 29, 2024 15:51
@chrstinalin chrstinalin removed the request for review from dotproto May 29, 2024 15:51
@chrstinalin chrstinalin requested a review from dotproto May 29, 2024 18:26
@chrstinalin
Copy link
Contributor Author

chrstinalin commented May 29, 2024

Note: All #31 and #43 need from here is the link on AMO to be changed as mentioned in the issue thread of #31. I don't know if it's actively being used, though, so it may be best to hold off on that change temporarily.

src/commands/getAddon.ts Outdated Show resolved Hide resolved
src/commands/getAddon.ts Outdated Show resolved Hide resolved
src/extension.ts Outdated Show resolved Hide resolved
Comment on lines +37 to +39
if (context.globalState.get("filePath") !== undefined) {
const filePath = context.globalState.get("filePath")?.toString();
const lineNumber = context.globalState.get("lineNumber")?.toString();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Setting this state and cleaning it up should be handled in a pair of functions on a relevant object. This might be a good fit for a general "AssayApp" class.

src/utils/commentManager.ts Outdated Show resolved Hide resolved
test/suite/commands/openFromUrl.test.ts Outdated Show resolved Hide resolved
test/suite/utils/revealFile.test.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@dotproto dotproto left a comment

Choose a reason for hiding this comment

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

LGTM. Nice work! :shipit:

@chrstinalin chrstinalin merged commit fa08f38 into main Jun 10, 2024
1 check was pending
@willdurand willdurand deleted the manifest-json branch July 9, 2024 11:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants