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

feat: add in conditional operator sample #414

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bd82
Copy link

@bd82 bd82 commented May 10, 2021

@bd82
Copy link
Author

bd82 commented May 10, 2021

@eamodio may be on interest to you as you implemented this feature in VS Code

@justinmk3
Copy link

1600 lines of code to demonstrate this feature is disproportionate. The relevant lines of code in this PR only 5-10.

@bd82
Copy link
Author

bd82 commented May 11, 2021

1600 lines of code to demonstrate this feature is disproportionate. The relevant lines of code in this PR only 5-10.

This PR is 1650 LOC:

  • 1350 LOC are the package-lock
  • 134 LOC is the extensions TS files (split into 3 files, with comments and with lots of blank lines for readability).
  • The rest is sample boiler plate or sample workspace for easy of use.

The one area where the LOC can be reduced is the 134 lines of the source code.
But I wanted to produce a more meaningful example which includes:

  • Two context menus, so the dynamic capabilities are emphasized.
  • Highlight the need to track the state of the custom contexts (fileWatcher for change / findFiles for initial state).
  • Include some basic custom logic to fill the custom contexts as the in operator is meant to allow such ext developer custom logic.

I believe a simplified sample of ~30LOC with some hard-coded values for the custom contexts
would not provide meaningful value (not really dynamic), while also not being that much easier to grok than 3 files of ~45 LOC average each...

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.

3 participants