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

tl.tl API cleanups and tl refactors #414

Merged
merged 7 commits into from Mar 11, 2021
Merged

tl.tl API cleanups and tl refactors #414

merged 7 commits into from Mar 11, 2021

Conversation

hishamhm
Copy link
Member

@hishamhm hishamhm commented Mar 6, 2021

tl.tl API changes

  • make unknowns a kind of warning — this means they can be silenced or turned into errors via the regular methods for warnings in the CLI
  • only ever return Result values, as their names imply, do not take them as inputs
  • each Result only contains errors for the given file
    • use env.loaded (and env.loaded_order) or result.dependencies to traverse all results and get all errors, as seen in the tl refactor below.

tl refactors

They're not as pretty as @euclidianAce's refactors in past recent PR, but this is more of a preparation for the deprecation of tl build and to use the API changes above.

  • move everything related to "tl build" to its own "submodule", preparing for deprecation (Cyan should be used as a build system instead)
  • moves all "top-level" code to the end
  • secondary quality-of-life changes while I was at it:
    • always look for tlconfig.lua in parent dirs (and use a dumber but faster method)
    • tl types: emit JSON map keys in alphabetical order — the order shouldn't matter, but it should make it easier to read when debugging

@github-actions
Copy link

github-actions bot commented Mar 6, 2021

Teal Playground URL: https://414--teal-playground.netlify.app

* move everything related to "tl build" to its own "submodule",
  preparing for deprecation (Cyan should be used as a build
  system instead)
* move (almost) all "top-level" code to the end (except for
  the global state definitions, which will be changed in another
  commit)
squeeze a little bit more of performance from the code
to compensate for the extra work in sorting map keys.
* make unknowns a kind of warning
* only ever return Result values, do not take them as inputs
* each Result only contains errors for the given file
* use env.loaded (and env.loaded_order) to traverse all results
  and get all errors
This was originally added in 2019 to be at the beginning of the
file, for the purpose explained in the comment below. Eventually
it ended up moving around in the file, which made it ineffective.
We've had reports of users confused that a git clone of tl was picking
up the wrong tl.lua, so this little trick seems to have value --
let's bring it back.
@euclidianAce
Copy link
Member

For what its worth (as I havent looked at the code much) I have a local branch of both cyan and teal-language-server up and running with this new api and they are both (seemingly) working. (And if anything isn't I'd blame that on my own test coverage rather than these changes)

@hishamhm
Copy link
Member Author

@euclidianAce Thank you for this feedback, it's super valuable.

I'll go ahead and merge this. Another thing I will add is to move the version string from the tl script into the API, so it can be checked by clients. 👍

@hishamhm hishamhm merged commit 17911e4 into master Mar 11, 2021
@hishamhm hishamhm deleted the tl-api-cleanups branch March 11, 2021 18:45
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