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: expose type of current selection via OpenAPI #1058

Merged
merged 1 commit into from
Jun 7, 2023

Conversation

dhess
Copy link
Member

@dhess dhess commented Jun 7, 2023

This is a rebase/rewrite of #1050

@dhess dhess marked this pull request as draft June 7, 2023 13:38
@georgefst
Copy link
Contributor

I've taken over rebasing this due to my familiarity with the Selection type etc. It should now be merge-able.

I've ended up changing the new API quite substantially, in line with #1050 (comment). This was the easiest solution, since we end up needing extra info that isn't directly available in the selection metadata, even for term definition name nodes (which, as it happens, we previously returned Nothing for, which would have been an issue for the frontend), and also we don't actually currently have a way to set a SelectionTypeDef via ProgAction (there's a bit of a weird inconsistency between type and term def actions, in that the former always take a selection, and the latter start from the current one - we should revisit this at some point).

I also made all of the previous Nothing cases throw errors directly, so the workaround implemented in 2885106 is no longer necessary.

This adds a new endpoint to the OpenAPI, but does not change any existing endpoints.

Signed-off-by: Ben Price <ben@hackworthltd.com>
Signed-off-by: Drew Hess <src@drewhess.com>
@dhess dhess marked this pull request as ready for review June 7, 2023 18:08
@dhess dhess changed the title Rebase of #1050 feat!: expose type of current selection via OpenAPI Jun 7, 2023
@dhess dhess changed the title feat!: expose type of current selection via OpenAPI feat: expose type of current selection via OpenAPI Jun 7, 2023
@georgefst georgefst force-pushed the dhess/api-type-rebase branch 2 times, most recently from 8edfbda to 38f72ef Compare June 7, 2023 18:12
@dhess dhess enabled auto-merge June 7, 2023 18:18
@dhess dhess disabled auto-merge June 7, 2023 18:20
@georgefst georgefst added this pull request to the merge queue Jun 7, 2023
Merged via the queue into main with commit 515d39d Jun 7, 2023
102 checks passed
@georgefst georgefst deleted the dhess/api-type-rebase branch June 7, 2023 19:27
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

2 participants