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

docs(bash-preexec): describe the limitation of missing commands #1937

Merged
merged 1 commit into from
Apr 9, 2024

Conversation

akinomyoga
Copy link
Contributor

I noticed that the problem of missing commands with bash-preexec is not mentioned in README. This PR adds a mention.

Also, the quick instruction at the beginning of README doesn't mention the possible minor problems with Bash integration, so I added a link to the later Bash section, which describes the problems in more detail.

As another change, I made those "notes" more outstanding using the GFM blockquote so that users do not miss them. There are still reports by users who missed those notes (like #1921).

Checks

  • I am happy for maintainers to push small adjustments to this PR, to speed up the review cycle
  • I have checked that there are no existing pull requests for the same thing

Copy link
Member

@ellie ellie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

may also be worth adding something to atuin doctor when it detects someone using bash-preexec

@ellie ellie merged commit 05becf9 into atuinsh:main Apr 9, 2024
17 checks passed
@akinomyoga akinomyoga deleted the bash-note branch April 9, 2024 11:44
@akinomyoga
Copy link
Contributor Author

Thanks!

may also be worth adding something to atuin doctor when it detects someone using bash-preexec

Yeah. Maybe a troubleshooting section can be added in README.

@ellie
Copy link
Member

ellie commented Apr 9, 2024

Yeah. Maybe a troubleshooting section can be added in README.

I'd rather add it to the docs and the doctor. My experience so far has been that people tend to ignore dense information - the docs has search, and if someone opens an issue without doing their own search, the doctor should catch that too

@akinomyoga
Copy link
Contributor Author

akinomyoga commented Apr 9, 2024

Ah, OK. I misread your previous comment! (I misunderstood you would be adding something related to atuin doctor in README.)

@akinomyoga
Copy link
Contributor Author

BTW, I have a commit akinomyoga@0598e4a for atuin doctor. It tries to check if Atuin's integration with bash-preexec or ble.sh is properly set up. Even if bash-preexec/ble.sh is loaded in the session, there is a possibility that the integration is not properly set up e.g. when the Atuin is initialized before ble.sh is sourced as in #1921. What do you think of this kind of detection?

@ellie
Copy link
Member

ellie commented Apr 9, 2024

What do you think of this kind of detection?

Seems reasonable to me!

@akinomyoga
Copy link
Contributor Author

Thank you! Then, I'll later submit a PR after some refinements.

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.

2 participants