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

Print current branch/database in REPL #1258

Merged
merged 4 commits into from
Mar 25, 2024
Merged

Conversation

quinchs
Copy link
Contributor

@quinchs quinchs commented Mar 22, 2024

Fixes #1250

@quinchs quinchs requested a review from aljazerzen March 22, 2024 19:27
Copy link
Contributor

@aljazerzen aljazerzen left a comment

Choose a reason for hiding this comment

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

This basically doubles the time between user hitting enter and CLI being ready for next command.

But there is no other way, so I guess ok.

@msullivan
Copy link
Member

This basically doubles the time between user hitting enter and CLI being ready for next command.

But there is no other way, so I guess ok.

Can't we fetch it only once at connection time?

@quinchs
Copy link
Contributor Author

quinchs commented Mar 22, 2024

Now only fetches at connection and if it's not cached

@aljazerzen
Copy link
Contributor

Oh, yes - branch cannot be changed with EdgeQL queries, so it can be cached for the whole duration of a connection.

That said: we could just the the "branch" connection parameter and to make any requests to the database. I guess this would be more fragile since we both "database" param and "branch" param and some non-trivial logic deciding which branch we actually open the connection to.

Bottom line: current approach is good

@aljazerzen
Copy link
Contributor

The interactive tests are failing because they expect to have "edgedb>" command prompt, but get "main>" instead.

@quinchs
Copy link
Contributor Author

quinchs commented Mar 25, 2024

The interactive tests are failing because they expect to have "edgedb>" command prompt, but get "main>" instead.

I'll fix that

@quinchs quinchs merged commit f6168f7 into master Mar 25, 2024
16 checks passed
@quinchs quinchs deleted the fix/print-current-branch-in-repl branch March 25, 2024 18:36
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.

We should populate the prompt with sys::get_current_database()
3 participants