-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Added comment to install "t" and disable "todo.plugin" #1742
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
Conversation
This commit is intended to help new uses of the modern-t theme get it set up properly and avoid the "todo.plugin" breaking it Please see the following issues for details. Bash-it#1693 Bash-it#1374
| # for "t", the minimalist python todo list utility by Steve Losh. | ||
| # Get and install "t" at https://github.com/sjl/t#installing-t | ||
| # | ||
| # Warning: The Bash-it plugin "todo.plugin" breaks the "t" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I prefer to fix the plugin and not add a warning here. I will try to fix it up in the upcoming days
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. I think "t" is pretty well unknown, and unless I missed it, I see no readme for the "modern" themes. That would be a better place for this info to be. Would a readme doc for the modern themes be appreciated? I have come this far, I'd like to try to get a PR in if I can. I would think it should be here "bash-it/docs/themes-list/". I also noticed the screenshots for "modern-t" also show that "t" is not installed, as there is an error. This is kind of what I mean, most that might try modern-t have no idea that there is an actual dependence called "t", or even if they figure it out, it's not easy to find, sadly as it is a very good little utility.
Also, It would be easy to create a "todo.sh" variant, "modern-todo" with the same CLI integration functionality as "modern-t". I'd be happy to submit that one, but I am unsure how people feel about having multiple varients of the same theme.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think adding a readme doc will be a great idea of you can do it- the location you assumed is the correct one as well!
I do not think we need another theme for that, but we might need to improve (and add) docs about "modern-t" and also maybe add a check that it indeed uses the "t" utility
|
Well, I am not sure what is wrong with the plugin in a technical sense. It seems there is just a collision between the plugins alias to "t" and that breaking the undocumented use of "t" the script in "modern-t". Now that I know this, I don't see any problem other than lack of documentation. If you know all these things, all it takes to get a working "modern-t" cli that work with todo.sh is to add Might be the theme could handle both properly by adding more "if then" statements to decide which function to use in the prompt based on what is available, but honestly better documentation for the theme and just putting these in a VAR with one commented out would be enough. OR... a whole new theme?
|
The plugin should not really create an alias like that, but in any case you are correct that the theme can check and behave differently if i.e t is an alias to something else. I will welcome PRs that improve the theme 😄 |
|
@russelltadams Thanks for this PR! I will merge this, but much more work is needed! |
Renames `aliases/available/todo.txt-cli.aliases.bash` to
`aliases/available/todo.aliases.bash` to match the naming convention
expected by the interactive install script.
**Problem:**
The install script in `_bash-it-install-enable()` (install.sh:26) parses
alias filenames using the pattern:
```bash
just_the_name="${file_name%".${file_type}.bash"}"
```
This expects: `{name}.aliases.bash` → extracts `{name}`
But the file was named `todo.txt-cli.aliases.bash`, which when stripped
of `.aliases.bash` leaves `todo.txt-cli`, causing the install script
to fail to recognize it during interactive installation.
**Solution:**
Rename to `todo.aliases.bash` to match the convention used by:
- `todo.plugin.bash` ✅
- `todo.completion.bash` ✅
Now extracts correctly: `todo.aliases.bash` → `todo`
**Notes:**
- File content unchanged - already uses `$TODO` variable correctly
- These are thin wrappers for the external todo.txt-cli tool
- Users install todo.txt-cli separately: `brew install todo-txt`
- Official upstream: https://github.com/todotxt/todo.txt-cli
**Related Issues:**
- Fixes Bash-it#2314 (interactive install failure)
- Related to Bash-it#1693, Bash-it#1742, Bash-it#2005 (todo plugin cleanup history)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
This commit is intended to help new users of the
modern-t theme get it set up properly
and avoid the "todo.plugin" breaking it
Please see the following issues for details.
#1693
#1374