org-mode icon indicating copy to clipboard operation
org-mode copied to clipboard

Feedback on Todo & Priority types

Open srid opened this issue 4 years ago • 2 comments

Heya,

Just trying out master and I have a couple of suggestions.

  1. ~~Since a priority can only be assigned to Todo headings, it would make sense to move it out of Section into Todo type.~~
  2. org-mode supports arbitrary number of todo states, so we might as well have a third constructor taking any Text in the Todo type, but the parser can accept it only if the first word is all capitalized (convention over configuration).

With these two applied, the Todo type would look like:

data TodoState = TODO | DONE | CustomTodo Text

data Todo = Todo 
  { todoState :: TodoState
  , todoPriority :: Maybe Priority
  }

I not even 100% confident of the TodoState type. I wonder if we should just use a newtype wrapper instead and have the library user code deal with the individual Todo states. After all, it is possible to set org-todo-keywords to something that does not even contain TODO or DONE keywords. So perhaps,

-- | The all-capitalized Todo keyword appearing as the first word in a section heading
newtype TodoState = TodoState { unTodoState :: Text }

data Todo = Todo 
  { todoState :: TodoState
  , todoPriority :: Maybe Priority
  }

srid avatar Apr 29 '21 16:04 srid

Also, since the rest of the newtypes in the library are defined without a field name, these types could just be:

newtype Priority = Priority Text 
newtype TodoState = TodoState Text

I'm happy to open a PR if you are busy otherwise, @fosskers

srid avatar Apr 29 '21 16:04 srid

Since a priority can only be assigned to Todo headings, it would make sense to move it out of Section into Todo type.

This isn't the case. You can assign priorities without a TODO marking, and they appear in the Agenda anyway.

fosskers avatar Apr 29 '21 17:04 fosskers