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

doctest allow int/line-number prefix #9430

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

S0AndS0
Copy link

@S0AndS0 S0AndS0 commented Feb 12, 2025

⚠️ nested case does not spark joy and RegExp might be a better choice for this feature request!

This patch aims to formally begin the discussion here about feature parity between Erlang OTP doctest, and the library of the same name maintained by williamthome

Specifically this patch targets > vs 1> syntax for declaring the start of a testable code line or set of lines, ex;

Erlang OTP doctest

-doc """
This test is how we do with `erlang/otp` doctests

```
> true.
true
> totallyOkay.
totallyOkay
```
"""
woot() -> ok.

williamthome doctest

-doc """
This test is how we do with `williamthome/doctest` (prior to version `0.10.0`)

```erlang
1> false.
false
2> thisIsFine.
thisIsFine
```
"""
muchJoy() -> ok.

⚠️ nested `case` does not spark joy and RegExp might be a better
choice for this feature request!

This patch aims to formally begin the discussion here about feature
parity between Erlang OTP doctest, and the library of the same name
maintained by [williamthome](https://github.com/williamthome/doctest/)

Specifically this patch targets `>` vs `1>` syntax for declaring the
start of a testable code line or set of lines, ex;

**Erlang OTP doctest**

````erlang
-doc """
This test is how we do with `erlang/otp` doctests

```
> true.
true
> totallyOkay.
totallyOkay
```
"""
woot() -> ok.
````

**williamthome doctest**

````erlang
-doc """
This test is how we do with `williamthome/doctest`

```erlang
1> false.
false
2> thisIsFine.
thisIsFine
```
"""
muchJoy() -> ok.
````
@CLAassistant
Copy link

CLAassistant commented Feb 12, 2025

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

github-actions bot commented Feb 12, 2025

CT Test Results

    2 files     97 suites   1h 8m 57s ⏱️
2 190 tests 2 143 ✅ 47 💤 0 ❌
2 556 runs  2 507 ✅ 49 💤 0 ❌

Results for commit c23347d.

♻️ This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

// Erlang/OTP Github Action Bot

@S0AndS0
Copy link
Author

S0AndS0 commented Feb 12, 2025

Here are some results of manual testing, because I'm not yet smort enough to run only doctests from the CLI;

  • Mixing of > and INT> seem fine, so I've not broken anything... yet
    > parse_tests([<<"> SomeCode.">>, <<"419> OtherCode.">>], []).
    [{test,[<<"SomeCode.">>,10,<<"OtherCode.">>],"_"}]
  • Multi-line code seems to work fine too
    > parse_tests([<<"68> ReallyLongLine,">>, <<" ConinuedOnAnotherLine.">>], []).
    [{test,[<<"ReallyLongLine,">>,10,
            <<"ConinuedOnAnotherLine.">>],
           "_"}]
    > parse_tests([<<"> ReallyLongLine,">>, <<" ConinuedOnAnotherLine.">>], []).
    [{test,[<<"ReallyLongLine,">>,10,
            <<"ConinuedOnAnotherLine.">>],
           "_"}]
  • Lines that contain > but not prefixed by an integer are ignored as usual
    > parse_tests([<<"Something > OtherThang.">>], []).
    [{test,[],[<<"Something > OtherThang.">>]}]
  • 🐛 Lines prefixed by any number of blank space are considered code
    > parse_tests([<<" wat.">>], []).
    [{test,[<<"wat.">>],"_"}]

    Note; double-checking against pre-PR code this is true there too 🤷

I still be learning the ways of Erlang, and least-bad practices, so any tips towards reducing future costs to maintenance are very much appreciated!

CC: @garazdawi I figured it'd be wise to nudge things specific to OTP parity from williamthome/doctest -- Issue 47 to here ;-)

@garazdawi
Copy link
Contributor

One of the things I like a lot about doctests is that it adds uniformity to the examples. So I think that I would like it to be very oppionionated about how you write examples. So I would like for the examples to either use > or 1>, not a mix. The 1> adds one extra unnecessary number in front, but it makes the examples closer to what you normally see in an Erlang shell.

Which one do you prefer and why?

@garazdawi
Copy link
Contributor

Here are some results of manual testing, because I'm not yet smort enough to run only doctests from the CLI;

1> shell_docs:test(ModuleToTest, []).

@S0AndS0
Copy link
Author

S0AndS0 commented Feb 13, 2025

Which one do you prefer and why?

For compatibility with preexisting doctest tooling and tests I prefer number prefixed code lines, also it should hopefully mean fewer examples need edited. Plus having an incrementing number makes it obvious to readers, and copy/pasters, of examples that there likely is an intended order. Ex;

1> State = "World".
2> Greeter = fun(State) -> "Hello " ++ State end.
3> Result = Greeter(State).
"Hello World"

At the same time I prefer >, without a line number prefix, because it could be used in doc-comments/tests where asserts are independent of one another, ex;

> true.
true
> false.
false

True having both is a more overhead to maintain and parse, and I'm but one opinionated noob ;-)

@williamthome
Copy link
Contributor

I think just the > is fine and less tedious. Elixir uses the iex> prefix and optionally allows the line numbers. See examples.

Maybe it would be nice to use erl> as the prefix to distinguish between Erlang and Elixir examples when they are on the same documentation, but I'm for the simplicity of the > prefix.


@garazdawi Sorry for the off-topic question below:

Is it possible for the shell to capture pasted content (Ctrl + p)? If so, it would be great if the shell could recognize the first character, and if it is a >, paste all the lines starting with the > in sequence. For instance:

```erlang
> A = 1.
1
> B = 1.
> A + B.
2
```

The shell would then print:

  1. A = 1.
  2. Enter
  3. B = 1.
  4. Enter
  5. A + B.
  6. Enter

It would be nice to have this feature to experiment with copied examples from the documentation.

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.

4 participants