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

Add echo keyword #3676

Open
wants to merge 46 commits into
base: main
Choose a base branch
from
Open

Conversation

giacomocavalieri
Copy link
Member

@giacomocavalieri giacomocavalieri commented Oct 7, 2024

This PR adds an implementation for the echo keyword and closes #2535.

It can be used to print any value:

pub fn main() {
  echo Wibble(1, 2, False)
}
// src/main.gleam:2 Wibble(1, 2, False) 

It can also be used in pipes:

pub fn main() {
  [1, 2, 3]
  |> echo
  |> list.map(fn(n) { n * 2 })
  |> echo
}
// src/main.gleam:3 [1, 2, 3]
// src/main.gleam:5 [2, 4, 6]

And returns the value being printed, so you can slap it everywhere if you need debug printing:

pub fn main() {
  let acc = #([], 1)
  list.fold([1, 2, 3], echo acc, fun)
}
// src/main.gleam:3 #([], 1)

Following from this discussion (#3625) here's a TODO list of things I have to do:

  • parse echo
  • pretty print echo
  • type echo
    • special handle an echo in the middle of a pipeline
    • make echo cannot be used without an expression unless it's as a step of a pipeline
    • write loads of tests
  • generate code for echo
    • copy io.debug's look
    • where should that code be?
    • How do I deal with Dict? The inspect code relied on importing Dict from stdlib but I can't do that
    • add other information like the location where the code comes from
  • make sure code with echo doesn't get published
  • deprecate io.debug from stdlib (deprecate io.debug in favour of echo stdlib#704)
  • add a page dedicated to echo in the language tour (Add Echo to language tour language-tour#132)
  • update syntax highlighting so that echo is highlighted as a keyword (Highlight echo tree-sitter-gleam#103)
  • add a warning for unreachable echo if someone writes echo panic
  • Write some integration tests!

And here's a list of things I've intentionally left out, that could be added in future PRs (for which we could open separate issues):

  • allow to add a tag with as "tag"
  • improve the pretty printing generated code based on type information (include labels, tell apart tuples from custom types, and strings from bitarrays)
  • add colours

@inoas
Copy link
Contributor

inoas commented Oct 8, 2024

Can we get the info from echo without actually pushing it to stdout/stderr for libs like pprint?

@lpil
Copy link
Member

lpil commented Oct 8, 2024

No.

@inoas
Copy link
Contributor

inoas commented Oct 8, 2024

No.

will string.inspect stay and just io.debug be gone?

  • if string.inspect stays - do we have two different implementations to inspect at runtime?
  • if string.inspect is also removed, would pprint require to impl string.inspect, pprint is very useful with birdie.

@lpil
Copy link
Member

lpil commented Oct 8, 2024

There will be no changes to string.inspect coming from this.

@giacomocavalieri
Copy link
Member Author

giacomocavalieri commented Oct 8, 2024

As far as I'm aware there's no plan to remove string.inspect, just io.debug since we don't want two ways to do the same things and just sticking to echo will give you guarantees like "the build tool won't let you publish a package with debug printing".

Also string.inspect and echo cover two different cases: the former is for getting a string from any piece of data, while the latter is for debug printing and nothing else.
It's output might change over time or diverge from what string.inspect is doing by taking advantage of type informations from the compiler, the discussion where we talked about it is here #3625

@inoas
Copy link
Contributor

inoas commented Oct 8, 2024

As far as I'm aware there's no plan to remove string.inspect, just io.debug since we don't want two ways to do the same things and just sticking to echo will give you guarantees like "the build tool won't let you publish a package with debug printing".

Also string.inspect and echo cover two different cases: the former is for getting a string from any piece of data, while the latter is for debug printing and nothing else. It's output might change over time or diverge from what string.inspect is doing by taking advantage of type informations from the compiler, the discussion where we talked about it is here #3625

Yes. It would just be nice to utilize what echo does in the long run for things like pprint etc instead of keeping 2 implementations around, imho.

Thank you for working on this <3

@lpil
Copy link
Member

lpil commented Oct 9, 2024

They will always have to be 2 implementations as they do 2 different things.

@graphiteisaac
Copy link
Contributor

wondering if it would be worth adding something to either the LSP or formatter to replace io.debug calls with echo, to ease the transition post deprecation? not a necessary thing by any means but could be a nice to have DX addition

@giacomocavalieri
Copy link
Member Author

The formatter cannot do this as it doesn't analyse your program and cannot know where that io.debug comes from (wether it's stdlib's io module or another module with the same name). We could have gleam fix do this but I'm not sure it's worth the extra work

@lpil
Copy link
Member

lpil commented Oct 18, 2024

Probably not worth the effort given io.debug calls tend to be removed after debugging is finished

@giacomocavalieri
Copy link
Member Author

This is still missing some integration tests to check the actual echo's output but I'd like to get some feedback on the code so far

Copy link
Member

@lpil lpil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!!! I've left some comments

I think it would be good to include the file and line number in the output of echo. Perhaps in grey on the line above the printed value? Somewhat similar to how stack traces show

edit: ah! Just noticed the todo list at the top of this PR 💜

compiler-cli/src/publish.rs Outdated Show resolved Hide resolved
compiler-core/src/ast.rs Show resolved Hide resolved
compiler-core/src/erlang/tests/echo.rs Outdated Show resolved Hide resolved
compiler-core/src/javascript.rs Outdated Show resolved Hide resolved
compiler-core/src/javascript/tests/echo.rs Outdated Show resolved Hide resolved
compiler-core/src/warning.rs Outdated Show resolved Hide resolved
compiler-core/templates/echo.mjs Show resolved Hide resolved
Copy link
Member

@lpil lpil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found a bug! I've pushed a couple of test cases.

The integration test lag is too much, we'll need to do something else there. I'm thinking we invoke the compiler-cli entrypoint directly from within Rust code rather than in a different OS process

Cargo.toml Outdated Show resolved Hide resolved
@inoas
Copy link
Contributor

inoas commented Nov 13, 2024

Ref: #3216 (comment)
Should echo stop gleam publish?

@GearsDatapacks
Copy link
Member

It does currently

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.

Debug printing feature in language
6 participants