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

Allow "importing" language facet support and tests #2123

Closed
1 task done
pokey opened this issue Dec 12, 2023 · 2 comments · Fixed by #2335
Closed
1 task done

Allow "importing" language facet support and tests #2123

pokey opened this issue Dec 12, 2023 · 2 comments · Fixed by #2335
Assignees

Comments

@pokey
Copy link
Member

pokey commented Dec 12, 2023

The problem

Our current facet support setup doesn't allow languages to share support info / tests. For example, there is a large subset of javascript and typescript that is the same. We'd like for both languages to indicate that there's a common core of things they both support, and to share the corresponding test cases

The solution

Let's use our .scm file import mechanism as inspiration. In our .scm files, there is a top-level file for each language id, which is allowed to import from other .scm flies. That way, both typescript.scm and javascript.scm can import from javascript.core.scm.

We should create a way to "import" support for our facets, including the tests, the way we do for our .scm files

We'll want to be able to have subdirectories in the scopes test fixture dir that don't correspond to language ids, so that we can put things like javascript.core in there.

We can do that by not iterating all top-level directories in the scopes test fixture dir, but instead look in the keys of our getLanguageSupport support info to figure out what languages to look for, and then look for directory with the language id as its name. That will enable us to add other dirs that are shared that don't correspond directly to a language id

Alternately, we could iterate all directories, but then look up in a table to see which languages import them

The next problem to solve is how to represent the imports. We need to represent both the support info, and the import of the tests

Option 1: represent them separately

Importing the support info

We could just use simple spread operator. Eg

const javascriptCore = { foo: supported, bar: supported, ...}
const javascript = { ...javascriptCore, bongo: supported }
const typescript = { ...javascriptCore, baz: supported }

Importing tests

Add support for an index.json file in language scope facet dir like

{
   "imports": ["javascript.core"]
}

Discussion

The advantage of this approach is that there's a bit less machinery, and it's fairly flexible. The disadvantage is that we have to represent the import info in two places

Option 2: Represent them together

We could create a table of imports in our Typescript representation, and use that both to pull support information from eg javascript.core and to automatically include tests

This approach is maybe a bit semantically richer, but requires more machinery, including creating "pseudo-languages" or something. If people like this approach, we can try to flesh it out, but it requires more thought than the other approach

Extra steps

Originally posted by @pokey in #2102 (comment)

@pokey pokey changed the title Allow "importing" language facet support Allow "importing" language facet support and tests Dec 12, 2023
@pokey pokey added the to discuss Plan to discuss at meet-up label Dec 12, 2023
@pokey
Copy link
Member Author

pokey commented Jan 4, 2024

Update from meet-up:

  • Go for option 1
  • Had several ideas for avoiding running the dirs that are not actually language id's and are just for importing:
    • Use a prefix or suffix on dir name, eg lib.javascript.core or javascript.core.lib, or even just if it has a . anywhere?
    • Put a config file in the directory indicating to skip it; could use same index.json file as specified above
    • Put it in a well-known subdir of scopes, eg lib
    • Put it in a different directory outside of scopes

Any leanings? cc/ @josharian

@pokey pokey removed the to discuss Plan to discuss at meet-up label Jan 4, 2024
@josharian
Copy link
Collaborator

No strong opinions based just on the descriptions above.

@AndreasArvidsson AndreasArvidsson self-assigned this May 14, 2024
github-merge-queue bot pushed a commit that referenced this issue May 17, 2024
Fixes #2123

## Checklist

- [x] I have added
[tests](https://www.cursorless.org/docs/contributing/test-case-recorder/)
- [ ] I have updated the
[docs](https://github.com/cursorless-dev/cursorless/tree/main/docs) and
[cheatsheet](https://github.com/cursorless-dev/cursorless/tree/main/cursorless-talon/src/cheatsheet)
- [/] I have not broken the cheatsheet

---------

Co-authored-by: Pokey Rule <755842+pokey@users.noreply.github.com>
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 a pull request may close this issue.

3 participants