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

Actually use user's ~/.psqlrc file #174

Merged
merged 1 commit into from
Oct 22, 2024

Conversation

rafbm
Copy link
Contributor

@rafbm rafbm commented Oct 17, 2024

Hey! Happy Bridge customer here. 👋😄

Since as long as I remember, my ~/.psqlrc customizations have never been available in cb psql sessions. Despite this commit, for instance, I think this probably never worked? Related changes in Crystal behavior seem to go a while back.

Here’s the fix, confirmed to work on my machine. (It’s awesome being able to test by just swapping cb for src/cli.cr!)

Given specs were green and still are, I assume that part just isn’t tested so didn’t study the test suite to find where to add it.

@rafbm rafbm requested a review from a team as a code owner October 17, 2024 14:41
@abrightwell
Copy link
Member

Hey @rafbm, thanks for the PR. I'll work on including it into the next release.

While I'm sure it is less common, it seems that we might want to also consider what might happen if the user were to make use of the PSQLRC environment variable. More specifically, if the location is outside of the user's home directory. I'll poke at it and let you know if I think we should include here.

I'm not terribly concerned with handling the version specific convention/naming at this time. But perhaps that's something to think about as well, just in case. I'll give that one more thought for later.

Also, this made me curious as to handling the -X, --no-psqlrc option if it were provided given: cb psql <ID> -- -X. Some initial testing seems to suggest that it's not a concern. And it's further supported given that psql will ignore loading/processing .psqlrc when this option is given which is confirmed here and here.

@rafbm
Copy link
Contributor Author

rafbm commented Oct 17, 2024

Great points! Feel free to amend this PR in any way. This was my first interaction with Crystal so you’ll sure be faster than me at any further improvements. 😄

@abrightwell abrightwell merged commit bf32370 into CrunchyData:main Oct 22, 2024
1 of 2 checks passed
@rafbm rafbm deleted the fix/user-psqlrc branch October 22, 2024 21:21
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