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

Propose Extractor API (#561, Plan A) #562

Merged
merged 23 commits into from
Mar 21, 2021

Conversation

bollwyvl
Copy link
Collaborator

@bollwyvl bollwyvl commented Mar 20, 2021

References

Code changes

  • in jupyterlab-lsp/index.ts, expose some minimal functional APIs from
    • jupyterlab-lsp/api/extractor.ts
    • jupyterlab-lsp/api/overrides.ts
  • adds _example-extractor to test the API
  • busy work while doing the above
    • fixes a couple linter warnings
    • normalize rimraf dependency across package.jsons
    • updates some flags for robotframework 4.0
    • add --preserveWatchOutput to watch

User-facing changes

  • n/a

Backwards-incompatible changes

  • n/a
    • authors of extensions for the new versions would be able to easily add extractor extensions, without shipping copies of the files (by deep referencing lib)

Chores

  • linted
  • tested
  • documented
  • changelog entry

@krassowski
Copy link
Member

Sorry for the mess in the commits!

@krassowski krassowski closed this Mar 21, 2021
@krassowski krassowski reopened this Mar 21, 2021
@bollwyvl
Copy link
Collaborator Author

I added a minimal package with a simple test to help keep us honest 😬

"lib/**/*.{js,ts}"
],
"dependencies": {
"@krassowski/jupyterlab-lsp": "^3.4"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"@krassowski/jupyterlab-lsp": "^3.4"
"@krassowski/jupyterlab-lsp": "^3.5"

Though the versions need to be bumped elsewhere for it to compile I think

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah, seems out of scope for this pr...

Copy link
Member

Choose a reason for hiding this comment

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

sure, can do it later, just noted to remember!

@bollwyvl
Copy link
Collaborator Author

still doin' some npm solvin'...

@bollwyvl
Copy link
Collaborator Author

Trying to roll back, but concerned the depedency chain of lsp-ws might be getting un-upgradeable...

export const extractor = new RegExpForeignCodeExtractor({
language: 'foo',
pattern: '^%%(foo)( .*?)?\n([^]*)',
extract_to_foreign: '$3',
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
extract_to_foreign: '$3',
foreign_capture_groups: [3],

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@bollwyvl
Copy link
Collaborator Author

Tests are looking good locally, no further changes planned...

@bollwyvl
Copy link
Collaborator Author

Seeing this locally, too:

'css:div.lsp-statusbar-item' not found.

@bollwyvl
Copy link
Collaborator Author

Ah, there's some bad entropy...

@bollwyvl
Copy link
Collaborator Author

Current plan (testing locally, before pushing):

  • revert packages/jupyterlab-lsp/src/tokens.ts
  • add a packages/jupyterlab-lsp/src/api/index.ts that gets exported in packages/jupyterlab-lspsrc//index.ts
  • add api/extractor.ts and api/overrides.ts, export these in the api/index.ts
  • hopefully not get circular import problems!

@bollwyvl
Copy link
Collaborator Author

Interactive testing looking good locally.

@bollwyvl
Copy link
Collaborator Author

Cool, some stuff is coming up ✔️

@bollwyvl
Copy link
Collaborator Author

aside from OSX 3.8 which is crapping out on master, all looks good!

@bollwyvl bollwyvl requested a review from krassowski March 21, 2021 22:34
@krassowski krassowski merged commit ffdd497 into jupyter-lsp:master Mar 21, 2021
@krassowski krassowski added this to the 3.5 milestone Aug 1, 2021
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.

Expose extractor API (as separate package?)
2 participants