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

todo command (by design, accidentally) flags references to todo and not just calls to todo. #5161

Open
aryairani opened this issue Jun 30, 2024 · 11 comments
Labels

Comments

@aryairani
Copy link
Contributor

Describe and demonstrate the bug

.> clone @unison/base
@unison/base/main> todo

  These terms call `todo`:
  
    1. todo.doc

@unison/base/main> view 1   
@unison/base/main> view #cvf3fk8t86n88d3p6q25sj6pgp9p00ie2m8nad75r0nijl152jsdnp2nnatf11hvg2usokg95dckve07dpvs1l509u9mf7ouub3lg88

  todo.doc : Doc
  todo.doc =
    use Text ++
    {{
    The {todo} function works the same way as the {bug} function in that it halts the program
    with a specific value, printing that value to the console.
    
    The main difference is that {todo} is intended to be used to communicate unfinished programs
    that still need work, where as {bug} communicates runtime invariants.
    
    **Example:**
    
    ``` ucm
    >todo (Some ?👋)
    ```
    
    The value will be printed out as the program halts:
    
    ``` ucm
    💔💥
    
    I've encountered a call to builtin.todo with the following value:
    
      Some ?👋
    
    I'm sorry this message doesn't have more detail about the location of the failure. My makers
    plan to fix this in a future release. 😢
    ```
    
    {todo} is useful for getting a program to typecheck without being fully ready to handle all
    cases of a sum type of instance:
    
    @typecheck ```
    hello : Optional Text -> Text
    hello = cases
      Some n -> "👋 Hello " ++ n
      None   -> todo "handle this case later"
    ```
    
    To get a list of todos, run the 'dependents todo' command in ucm.
    }}

Environment (please complete the following information):

  • v0.5.23
@aryairani aryairani added the bug label Jun 30, 2024
@mitchellwrosen
Copy link
Member

What's the difference between a reference to todo and a call to todo?

@ceedubs
Copy link
Contributor

ceedubs commented Jul 1, 2024

Before much time/effort is put into addressing this, I think that it's worth considering how useful it is to make the todo command find calls to the todo term. To me unnamed dependencies and conflicts are things that I want to address right away and are likely on a different lifecycle than todo "handle exception that we don't think will ever actually be raised by Instant.now". If I want to find these, it's easy enough to call dependents todo, and I'm not necessarily sure that I want todo to flag them. Though admittedly based on the command name it would be a reasonable expectation.

Basically I don't think that I care too much whether the todo command finds references to the todo term and I don't think that it would be worth an extended rabbit hole of trying to circumnavigate docs that mention todo.

@aryairani
Copy link
Contributor Author

What's the difference between a reference to todo and a call to todo?

Probably saturated vs unsaturated

@aryairani
Copy link
Contributor Author

todo "handle exception that we don't think will ever actually be raised by Instant.now"

this should probably call bug, and not todo?

Even if we take it out of todo and stick with dependents todo, we still have the issue of distinguishing between calling todo vs just talking about calling todo, in some doc. Or do we just say that they tend to all be one case or all be the other case, depending on whether the project in question is website? I could believe that

@mitchellwrosen
Copy link
Member

Probably saturated vs unsaturated

Hmm, but what about e.g. List.map todo [1,2,3]?

@aryairani
Copy link
Contributor Author

Lol, okay. Maybe we're back to "filter out docs from the output"?

@mitchellwrosen
Copy link
Member

mitchellwrosen commented Jul 2, 2024

I feel that would perhaps be worse than the status quo – under-reporting legitimate todo as opposed to over-reporting.

I definitely agree that it's obnoxious and wrong to call out to base developers that their todo.doc is in need of attention. But that may well be the only such false positive anyone will ever realistically see – what other developer would have reason to link to todo in a doc?

Not that we can't come up with an accurate solution – I think we can, with enough muscle – but just filtering out all Doc-typed terms from todo dependents would obscure things like

foo.doc = {{
  Foo is a thing, check it out:
  
  ```
  foo 5 (todo "finish this example")
  ```
}}

which would be unfortunate.

@aryairani
Copy link
Contributor Author

The website has a bunch of demonstrations of todo in example code; maybe @rlmark will forgive us though.

@aryairani
Copy link
Contributor Author

aryairani commented Jul 2, 2024

but just filtering out all Doc-typed terms from todo dependents would obscure things like

foo.doc = {{
  Foo is a thing, check it out:
  
      ```
      foo 5 (todo "finish this example")
      ```
}}

which would be unfortunate.

Yeah this is exactly the sort of thing I imagine a doc writer could want to do, and consider it done, not non-done.

@mitchellwrosen
Copy link
Member

Okay, todo's own doc plus all docs that we've authored to demonstrate its usage :) Not one doc, but a confined set nonetheless.

Yeah this is exactly the sort of thing I imagine a doc writer could want to do, and consider it done, not non-done.

Oh? That looks non-done to me... the evaluation will just barf rather than show off what foo does. Maybe we need to flesh out the example better or pick a different one.

@aryairani
Copy link
Contributor Author

the evaluation will just barf rather than show off what foo does.

Oops, I didn't notice that. If it's being evaluated, I'd agree it's non-done. If it's just being displayed and not evaluated, then I think that should be allowed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants