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

⌨️ No way to generate spans for input strings at an interactive prompt #87

Closed
favonia opened this issue Oct 7, 2023 · 3 comments · Fixed by #90
Closed

⌨️ No way to generate spans for input strings at an interactive prompt #87

favonia opened this issue Oct 7, 2023 · 3 comments · Fixed by #90
Assignees
Labels
bug Something isn't working enhancement New feature or request proposal
Milestone

Comments

@favonia
Copy link
Contributor

favonia commented Oct 7, 2023

Yet another great question/suggestion from @mikeshulman. The library is expecting a file_path in a position, but there's no "file" at interactive prompts. While the internal code is highly modularized and file I/O is isolated, the current external API bundles the file I/O. Here are some concrete fixes I can think of:

  1. Rename Position.file_path to Position.source and make its type polymorphic (so that we will have 'source Span.t). Downsides: too many type variables can reduce usability.
  2. Still rename Position.file_path to Position.source but fix its type to the following:
    type source = [`File of string | `String of string | `Uri of string ]

In either way, a more generic backend which can handle strings from interactive prompts should be made available. This can be as simple as taking yet another optional argument to handle URIs (if we want to allow them).

@favonia favonia changed the title 🔦 Highlight input strings at an interactive prompt 🔦 No way to highlight input strings at an interactive prompt Oct 7, 2023
@favonia favonia changed the title 🔦 No way to highlight input strings at an interactive prompt ⌨️ No way to highlight input strings at an interactive prompt Oct 7, 2023
@favonia favonia changed the title ⌨️ No way to highlight input strings at an interactive prompt ⌨️ No way to generate spans for input strings at an interactive prompt Oct 7, 2023
@favonia favonia added bug Something isn't working enhancement New feature or request proposal labels Oct 7, 2023
@favonia favonia added this to the 0.2.0 milestone Oct 7, 2023
@favonia favonia self-assigned this Oct 7, 2023
@mikeshulman
Copy link
Collaborator

I would probably go for (2) myself. Although instead of

type source = [`File of string | `String of string | `Uri of string ]

I would have been inclined to write

type source = File of string | String of string | Uri of string

Is there a reason to use polymorphic variants here?

@favonia
Copy link
Contributor Author

favonia commented Oct 7, 2023

I would have been inclined to write

type source = File of string | String of string | Uri of string

Is there a reason to use polymorphic variants here?

My main hesitation consists of two parts:

  1. Having very generic constructors such as File and String might pollute the constructor namespace. OCaml has type-directed disambiguation, which means it will consider all constructors in scope and others indicated by the types (this might not be accurate because I didn't check its algorithm). Polymorphic variants can stop OCaml from insisting on pinning down the "correct" type. I also think that if an implementer (the one who is building the application) wants to declare such generic constructors, so be it. They can pollute their own constructor namespace. But I tend to be more cautious when designing a library (that might pollute the namespace in all the applications using it).
  2. The API should have provided enough helper functions to hide such nasty details in common cases. Perhaps a reasonable design will emerge after we resolve the issue 🧮 Helper functions to recover byte offsets? #88. If this is the case, the burden will be invisible to implementers.

@mikeshulman
Copy link
Collaborator

Interesting, thanks. I was not consciously aware of that issue with non-polymorphic variants, although I have been annoyed sometimes at OCaml getting confused when I have constructors in different datatypes with the same name, so maybe I should use polymorphic variants more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request proposal
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants