Skip to content

Add the generate-examples library #940

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

Closed
wants to merge 3 commits into from

Conversation

SimeonC
Copy link
Contributor

@SimeonC SimeonC commented Sep 8, 2022

Have issues with the selections not quite being generated and inserted correctly (recursive selections are kinda hard).

Current output of the buildDictionary script is being manually ported into https://codepen.io/SimeonC/pen/gOeBJLr

@pokey
Copy link
Member

pokey commented Dec 8, 2022

Hi @SimeonC, just checking in on this one to see:

  • What still needs to happen on this PR?
  • Do you have any bandwidth to work on it, or should we take it over?

Thanks!

Fwiw I resolved merge conflicts so make sure to pull if you start hacking again 😊

@SimeonC
Copy link
Contributor Author

SimeonC commented Dec 9, 2022

Hi @pokey. Sorry for the radio silence, had a 3-month performance refactor that sapped all my mental energy and time. That's over and finished and I was hoping to get back and figure this out a bit more at some point.

For what's left, I think it's the following;

  • type issues
  • some bugs in the generation around creating selections
  • moving in the CSS code from the codepen (and completing it for all the cases)
  • define some kind of internal API so it can be called properly or something like that

I may not have any bandwidth until next year due to Christmas leave (unless I get some time next week). I definitely wanted to finish this - if you guys want to take it over and finish it to get it on the website I don't mind, I'm not so attached to it to block you guys from getting the features out you want.

@pokey
Copy link
Member

pokey commented Dec 9, 2022

Hi @SimeonC. No worries; just wanted to check in. We have other stuff to focus on at the moment, so wouldn't pick it up until next year anyway. Happy to leave it with you for the time being

There is an issue here that the jest parser requires no file name extension in imports, but ts-node requires `.js` extension. Probably will need to change this to build then generate rather than ts-node (or spend ages messing with the configs)
@SimeonC SimeonC force-pushed the generate-examples branch from b072615 to 070c870 Compare March 20, 2023 01:18
@SimeonC
Copy link
Contributor Author

SimeonC commented Mar 20, 2023

I'm still alive!! 🤣 Finally found some time and realised that my old code was so bad that there was no way I wanted to figure out what was going on so I started again.
In the interim at work we have been looking through the "Clean Coder" so I've applied some of that - primarily using classes, smaller functions and TDD. Which should make this PR overall much better to review.

@pokey
Copy link
Member

pokey commented Mar 20, 2023

Nice! Will have a look. Fwiw we're about to be on a proper pnpm monorepo; expecting #1313 to be merged in next 48hrs

@SimeonC
Copy link
Contributor Author

SimeonC commented Mar 20, 2023

Oh, sorry - must have accidentally deleted the line that said that this still needs work - a couple more evenings when I get the chance. Mostly wanted to say I am still trying to find time to get this done and haven't just gone silent and ghosted you all.

@pokey
Copy link
Member

pokey commented Mar 20, 2023

ah ok cool lmk when you want me to take a look; i'm happy to give early feedback if helpful

@pokey
Copy link
Member

pokey commented Mar 21, 2023

Ok we're now a pnpm monorepo. The cursorless-nx subdir is gone. You'll want to put your package in the packages directory. See https://www.cursorless.org/docs/contributing/adding-a-new-package/

Also note that you can now import from the cursorless codebase, which should make your life easier

@pokey
Copy link
Member

pokey commented Mar 24, 2023

Re importing packages from the rest of Cursorless codebase, any package that doesn't have vscode in the name is fair game to import for your build pipeline. If you need to import one of the packages into browser runtime context, let me know; we are planning to abstract away references to node but haven't gotten there yet

Need to update to be in the new NX repo format and correctly handle the newer formats for test fixtures which have changed since this was written
@pokey
Copy link
Member

pokey commented Aug 8, 2023

@SimeonC lives! 😄

Good to see you hacking again. A lot has changed in this repo since you last merged up 😅. Maybe worth dropping into a meet-up to discuss?

Note that we now have a spoken form generator that you could just use, but not sure it's necessary anymore as all of the spoken forms in our test cases have now been canonicalised, so I think you could just use the spoken form from the test case

@SimeonC
Copy link
Contributor Author

SimeonC commented Aug 8, 2023

😆 Yea, I finally found some time. I've just been hacking away at the generation part and I can go figure out how to update everything to fit the updates.

@pokey
Copy link
Member

pokey commented Aug 8, 2023

Fwiw we've also started just upgrading text fixture command payloads on the fly if we need the latest, so you should be able to do that to have your code work on legacy test fixtures.

@trillium trillium mentioned this pull request Mar 25, 2024
9 tasks
@SimeonC SimeonC closed this Mar 26, 2024
trillium pushed a commit to trillium/cursorless that referenced this pull request Mar 27, 2024
wip: Delete workspace.json for cherry-pick
- git cherry pick 070c870
- Delete cursorless-nx/workspace.json
Add the package and initial working

There is an issue here that the jest parser requires no file name extension in imports, but ts-node requires `.js` extension. Probably will need to change this to build then generate rather than ts-node (or spend ages messing with the configs)

[pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

All working with tests

Need to update to be in the new NX repo format and correctly handle the newer formats for test fixtures which have changed since this was written
trillium pushed a commit to trillium/cursorless that referenced this pull request Oct 3, 2024
wip: Delete workspace.json for cherry-pick
- git cherry pick 070c870
- Delete cursorless-nx/workspace.json
Add the package and initial working

There is an issue here that the jest parser requires no file name extension in imports, but ts-node requires `.js` extension. Probably will need to change this to build then generate rather than ts-node (or spend ages messing with the configs)

[pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

All working with tests

Need to update to be in the new NX repo format and correctly handle the newer formats for test fixtures which have changed since this was written
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.

2 participants