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

Rename supertype identifiers #291

Closed
wants to merge 2 commits into from

Conversation

314eter
Copy link
Contributor

@314eter 314eter commented Sep 8, 2020

This solves the problem mentioned in #290. Supertypes with similar names to other nodes (e.g. _sequence_expression and sequence_expression) result in the same identifier (SequenceExpression).

I solved this by prefixing names that start with _ with Super. These are always supertypes, since other names starting with an underscore are invisible in the AST.

Some examples of the new behaviour:

toHaskellCamelCaseIdentifier "expression" = "expression"
toHaskellCamelCaseIdentifier "sequence_expression" = "sequenceExpression"
toHaskellCamelCaseIdentifier "_sequence_expression" = "superSequenceExpression"
toHaskellCamelCaseIdentifier "__sequence_expression" = "superSuperSequenceExpression"
toHaskellCamelCaseIdentifier "__sequence__expression" = "superSuperSequenceUnderscoreExpression"

toHaskellPascalCaseIdentifier "expression" = "Expression"
toHaskellPascalCaseIdentifier "sequence_expression" = "SequenceExpression"
toHaskellPascalCaseIdentifier "_sequence_expression" = "SuperSequenceExpression"
toHaskellPascalCaseIdentifier "__sequence_expression" = "SuperSuperSequenceExpression"
toHaskellPascalCaseIdentifier "__sequence__expression" = "SuperSuperSequenceUnderscoreExpression"

@XVilka
Copy link
Contributor

XVilka commented Jun 25, 2022

Almost two years have passed. Was there any decision on this yet? cc @robrix

@robrix
Copy link
Contributor

robrix commented Jun 29, 2022

@XVilka apologies, this did indeed slip through the cracks. I'd like to see better names for the inner functions (I only use go when there's only the one such, and even then it is probably a bad habit).

Are you running into the problem this resolves? Can you confirm that this PR fixes it for you?

@robrix robrix closed this Jun 29, 2022
@robrix robrix reopened this Jun 29, 2022
@robrix
Copy link
Contributor

robrix commented Jun 29, 2022

Whoops, didn't mean to close this.

@robrix robrix closed this Jun 29, 2022
@robrix robrix reopened this Jun 29, 2022
@robrix
Copy link
Contributor

robrix commented Jun 29, 2022

🤣

@314eter
Copy link
Contributor Author

314eter commented Jun 29, 2022

I opened this this PR to fix a problem I encountered in github/semantic#623. It's still needed to get OCaml support in semantic (and probably for some other languages too). But since this was apparently not the way to get code navigation support in Github, I never finished it.

@robrix
Copy link
Contributor

robrix commented Jul 11, 2022

@314eter @XVilka Given that, any objection to me closing this out?

@XVilka
Copy link
Contributor

XVilka commented Jul 11, 2022

@robrix no objections from my side, quite sad it's not used anywhere now though...

@robrix robrix closed this Jul 11, 2022
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