Improve list/read command behavior and cleanups#1
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8411a4bd70
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| in | ||
| let displayed_position = match sort_order with | ||
| | Traditional -> (fun item -> item.traditional_order) | ||
| | Alphabetical -> (fun item -> item.alphabetical_order) |
There was a problem hiding this comment.
Number books by sorted index, not
alphabetical_order
Using item.alphabetical_order as the displayed numeric column makes bible list books --alphabetical output ambiguous, because those values are not unique in the data model (for example, I Samuel and Mark both use 28, and several books share 25 in lib/models/models.ml). This change introduces repeated/missing numbers in the left column, so users can no longer treat it as a position indicator; computing the number from the book's index in the sorted list would keep the column stable and meaningful.
Useful? React with 👍 / 👎.
Motivation
listcommand prints the "books" header only when listing books and not when listing translations.Description
list_booksto compute adisplayed_positionaccessor based on the chosensort_orderand use it when rendering the numeric column."The books of the Bible, sorted %sly:") into theBooksbranch so it is not printed forTranslations.build_chapterinlib/commands/cmd_read.mlby removing an unnecessaryOptionwrapping/fold around chapter content that is always present.Str.global_replacecalls with a single regex"\n\n\n+"normalization and remove the temporaryList.take 2cap so full-Bible reads process all fetched books.vreses->verses).Testing
git diff --checkwhich reported no whitespace/format errors and succeeded.rg "List.take 2|vreses|The books of the Bible"which confirmed the intended changes are present.dune buildin the environment which failed becauseduneis not installed, so compilation could not be validated here.Codex Task