DRY Available.options, Action.toProgActionNoInput and Action.toProgActionInput
#1041
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: George Thomas <[email protected]>
This will be necessary in order to make use of them in actions-related modules. Signed-off-by: George Thomas <[email protected]>
We parameterise this way, rather than parameterising `ExprMeta` and `TypeMeta` separately, since we otherwise wouldn't be able to reconstruct even a basic `Selection' ID` from the OpenAPI API, without clients knowing whether a particular ID corresponds to a type or term. We want clients to be able to be dumber than that. Note that, for `Selection`, we use a synonym for the non-parameterised version, but not for `NodeSelection`, and we will not in future for `DefSelection` and `TypeDefSelection`. That's because this synonym is actually widely useful, and we use it in several modules to make things more readable. With `NodeSelection` etc. we never really require such a synonym, and we avoid it because of all the ceremony it would add, particularly around imports. Signed-off-by: George Thomas <[email protected]>
This will significantly decrease the amount of breakage when we unify our selection types. Signed-off-by: George Thomas <[email protected]>
This removes some boilerplate where we converted between types which are essentially the same. That boilerplate would have become much bigger once we extend selections to cover type definitions. Despite the previous commit, there is one small breaking change, seen in `openapi.json` (`meta` instead of `id`). Signed-off-by: George Thomas <[email protected]>
Signed-off-by: George Thomas <[email protected]>
Signed-off-by: George Thomas <[email protected]>
This will make it easier to output typedefs in the API, due to increased uniformity with AST typedefs. Note that these names aren't even currently used anywhere since we don't actually have any higher-kinded primitives, only ints and chars. These would be used if we added, for example, `IO` or `Array` primitives, in which case the names would be likely useful at least for _displaying_ primitive typedefs, even though they don't actually scope over anything like they do for ASTs. Signed-off-by: George Thomas <[email protected]>
Signed-off-by: George Thomas <[email protected]>
Note that no new unit tests are added. This is because the underlying `ProgAction`s are already well-tested, e.g. in `unit_RenameType`. In due course, the property test `tasty_available_actions_accepted` will be generalised to cover typedef actions, which will therefore check that all of these new actions can be applied without error whenever they are available. We don't yet expose actions for constructor fields (i.e. `Available.forTypeDefConsFieldNode` always returns `[]`), since making this work will, unlike the other positions, require changes to the core of the library. We _could_ actually make all the `for*`s part of one definition, now that `Selection` is in `App.Base`. But we do actually use the individual functions in some tests, and with them separate, we have slightly more control, in that we don't need to provide as much context. Note that most of the changes in this commit are actually knock-on effects of generalising `Selection` to cover type defs. Signed-off-by: George Thomas <[email protected]>
This was motivated by a surprising deep equality failure on selections in a TypeScript frontend, due to a null field only present in one of the compared values. Signed-off-by: George Thomas <[email protected]>
Signed-off-by: George Thomas <[email protected]>
Signed-off-by: George Thomas <[email protected]>
`Available.options`, `Action.toProgActionNoInput` and `Action.toProgActionInput` Signed-off-by: George Thomas <[email protected]>
georgefst
force-pushed
the
georgefst/typedef-actions-dry-toprogaction
branch
from
798e08e
to
5a52fcd
Compare
georgefst
force-pushed
the
georgefst/typedef-actions
branch
2 times, most recently
from
f353179
to
c42e031
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
An addendum to #949. See #949 (comment).
These functions had gotten a bit repetitive. Unfortunately my first idea for factoring out this repetition ended up getting pretty messy. I may be missing a better way. I'll revisit this when there's less urgency to implement other features.