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

Import/Export pets #308

Merged
merged 12 commits into from
Nov 21, 2022
Merged

Import/Export pets #308

merged 12 commits into from
Nov 21, 2022

Conversation

Harry-Hopkinson
Copy link
Contributor

@Harry-Hopkinson Harry-Hopkinson commented Nov 5, 2022

Implementing a Pair Programming Feature with a text file for #237

@Harry-Hopkinson Harry-Hopkinson marked this pull request as draft November 5, 2022 20:47
@Harry-Hopkinson Harry-Hopkinson changed the title Pair Programming #237 Pair Programming Nov 5, 2022
@Harry-Hopkinson Harry-Hopkinson marked this pull request as ready for review November 6, 2022 17:40
@Harry-Hopkinson
Copy link
Contributor Author

@tonybaloney this pull request should be ready for a review.

.eslintrc.json Outdated Show resolved Hide resolved
@tonybaloney
Copy link
Owner

File imports will be another blocker to #276 in which browser extensions cannot support the fs module.

Please have a look at https://www.npmjs.com/package/vsls with example https://marketplace.visualstudio.com/items?itemName=lostintangent.vsls-whiteboard as APIs for sharing information inside a VS Code live share.

@codecov-commenter
Copy link

codecov-commenter commented Nov 6, 2022

Codecov Report

Merging #308 (fb9aa72) into master (fe1b63f) will decrease coverage by 3.50%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #308      +/-   ##
==========================================
- Coverage   16.22%   12.72%   -3.51%     
==========================================
  Files          19       16       -3     
  Lines        1103      731     -372     
  Branches      159      100      -59     
==========================================
- Hits          179       93      -86     
+ Misses        916      638     -278     
+ Partials        8        0       -8     
Impacted Files Coverage Δ
src/extension/extension.ts
src/common/localize.ts
src/common/names.ts

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@Harry-Hopkinson
Copy link
Contributor Author

File imports will be another blocker to #276 in which browser extensions cannot support the fs module.

Please have a look at https://www.npmjs.com/package/vsls with example https://marketplace.visualstudio.com/items?itemName=lostintangent.vsls-whiteboard as APIs for sharing information inside a VS Code live share.

Does vscode's built in file system work in the browser?

@tonybaloney tonybaloney changed the title Pair Programming Import/Export pets Nov 21, 2022
@tonybaloney
Copy link
Owner

I've made a couple of changes:

  • The exported files are JSON instead of the text format. The array type was already serializable and now it'll handle the formatting issues
  • The input colour is validated to avoid the mystery pet bug
  • The exported file uses the "untitled" protocol which opens it in a tab in VS Code ready to save, but without having saved the files so the user can rename it
  • The exported file name has a unique-ish number in it to avoid collision
  • The import action uses the open file dialog so the user can pick the file, instead of assuming it's named a certain way and inside the workspace (since the workspace might be a project they're working on and nothing to do with pets).

@tonybaloney tonybaloney merged commit 67ac6ca into tonybaloney:master Nov 21, 2022
@Harry-Hopkinson Harry-Hopkinson deleted the pair-programming branch November 23, 2022 21:29
@Harry-Hopkinson
Copy link
Contributor Author

@tonybaloney thank you

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.

None yet

3 participants