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

Changes to behavior of SQL output pane #157

Closed
richb-hanover opened this issue Mar 23, 2023 · 10 comments
Closed

Changes to behavior of SQL output pane #157

richb-hanover opened this issue Mar 23, 2023 · 10 comments

Comments

@richb-hanover
Copy link
Contributor

I'm using the 0.6.1 version of the VS Code extension. If I open a PRQL query from a file, then hit Cmd-Shift-P to open the PRQL Extension, the right half of the window fills with the generated SQL as expected.

But if I open another PRQL query file in the left half, the SQL in the right side does not change.

My impression is that this is different from previous versions, where changing the PRQL query displayed changes the SQL Output window. Or am I simply mistaken... Thanks.

@max-sixty
Copy link
Member

Yes, this seems to happen — the preview is now linked to a specific file rather than being generic for whichever file was last open.

Without understanding all the tradeoffs, I marginally preferred the old approach (i.e. possibly there are other things this approach enables).

One other thing I see — with two files open, each with previews available, when switching between PRQL files, the focus is grabbed by the preview. Ideally the preview would never grab focus (there's not much we can do with it...)

@richb-hanover
Copy link
Contributor Author

I wanted to follow up on my initial report. After playing with it more, I finally realized that the 0.6.1 version is designed to make it easier to preserve the SQL results from the PRQL query.

I strongly prefer the prior (0.6.0?) behavior. In my workflow, I like to treat the PRQL query files as the sole "source of truth". I don't find a need to keep the SQL results. They're just interim data that I copy/paste into my database.

Here are the features that I appreciate/treasure from the extension, and how I'd like to operate again:

  • The SQL Preview Pane simply displays the current PRQL file.
    • When I open the SQL Preview Pane, it displays the results of the current PRQL file open on the left.
    • Clicking another tab on the left or opening a new .prql file should switch the SQL Preview Pane to show the result of that query.
  • I like the "Copy SQL to Clipboard" icon at the top
  • For myself, I don't need either the "Generate SQL File" icon at the top (I don't preserve SQL results), or the "Open SQL Preview" icon (since it would always be present)
  • I like the "Copied SQL to Clipboard" confirmation. It can go away automatically after ~5-10 seconds.

Thanks.

@RandomFractals
Copy link
Collaborator

RandomFractals commented Mar 27, 2023

Sql Preview has been refactored and changed to support multiple PRQL document output views, serialization, and restore on vscode reload and many other features centered around individual PRQL documents linked to the corresponding SQL Preview views rather than using a single view as before. See #60 and #108 for more info and changed behavior.

PRQL VSCode v0.6.0 changelog covers those and other major changes made to how the old simplistic SQL Preview functioned: https://github.com/PRQL/prql-vscode/releases/tag/0.6.0

@richb-hanover
Copy link
Contributor Author

richb-hanover commented Mar 29, 2023

In the refactoring, I have lost the simplicity of earlier versions. Here's the result I would like: when I open any ".prql" file, I want to copy the SQL and paste it into my database. This means:

  • The "Copy SQL to Clipboard" icon would always be active.
  • The SQL Preview pane doesn't need to be open/visible. (If I want to view the SQL, I can always open the preview pane)
  • All the rest of the icons (Generate SQL File, Open SQL Preview, View PRQL Setting) can remain the same

I am hopeful that this is a straightforward change. Thanks again for a terrific tool.

@richb-hanover richb-hanover changed the title Did behavior of SQL output pane change? Changes to behavior of SQL output pane Mar 29, 2023
@RandomFractals
Copy link
Collaborator

@richb-hanover That is not how Copy SQL to Clipboard fucntionality was designed, and that functionality did not exist in prior version. You need an open SQL Preview to Copy generated SQL from it. You can also use Generate SQL file that opens the resulting SQL you can copy from a code editor and not commit that sql to your project repo, if you don't want to keep sql files in your project.

In any case, none of that Copy SQL or generate SQL file functionality was in the prior vscode extension v0.4.x you are referencing, and those changes were not made to remove the "simplicity" of the earlier versions. They are both new features and shortcuts added to work with PRQL in VSCode. Also, current SQL Preview works as most Previews work in other custom extensions, providing a preview for the open/active document.

You can review those changes in PR #49, #59, #118 with demo gifs if it's still not clear how that functionality works.

We moved on from PRQL code tools and it's up to @max-sixty and other devs contributing here if they want to rollback those features and added functionality, or close this ticket with the clarifications of those features we've provided here.

@max-sixty
Copy link
Member

Just to level-set on expectations:

  • Overall the extension added a lot of features as a result of @RandomFractals 's work. I think it's much much better, I'm grateful for the contribution.
  • As with any change, as we explore the space we might regress on some dimensions. That's why this sort of feedback from @richb-hanover is valuable
  • I think there's space to re-introduce the existing "single generic preview pane" output that we had before, while keeping the new features
  • My understanding is that @RandomFractals is working less on the extension now
  • Others are welcome to contribute! That's the nature of open-source... If @RandomFractals wants to, he's of course very welcome to, but zero obligation

@richb-hanover
Copy link
Contributor Author

richb-hanover commented Mar 30, 2023

Thank you @max-sixty for these thoughts. I just saw the comment from @RandomFractals and wanted to say that I didn't intend to seem as grumpy argumentative as what I wrote. Sorry.

My sole request is that, going forward, the "Copy SQL to Clipboard" be active whenever a .prql file has focus, whether the SQL Preview pane is open or not.

Thanks for listening.

@RandomFractals
Copy link
Collaborator

RandomFractals commented Mar 31, 2023

Thank you @max-sixty for these thoughts. I just saw the comment from @RandomFractals and wanted to say that I didn't intend to seem as grumpy argumentative as what I wrote. Sorry.

My sole request is that, going forward, the "Copy SQL to Clipboard" be active whenever a .prql file has focus, whether the SQL Preview pane is open or not.

Thanks for listening.

That would be a new feature request. I suggest we close this ticket, and create one for generating SQL in Copy Sql to Clipboard for an open PRQL document in an active vscode editor, without an open SQL Preview.

Also, even though we explicitly set no focus in open SQL Preview, for some reason it steals focus from an active PRQL document. It's an issue @max-sixty discovered and I was able to reproduce. Worth for other devs contributing here to investigate and patch.

@richb-hanover I did not take your comments as argumentative. Perhaps my summary sounded as one. I just wanted to clarify the history of those changes, and I know that changing workflow due to tool UI changes can be sometimes unexpected and somewhat frustrating. I do think those shortcuts made PRQL more usable in vscode, and I would recommend new sql gen/copy to clipboard feature is added per your request without resorting to a single SQL Preview webview. Only the most basic vscode extensions do that. Take md docs for example. I like how that built-in vscode Preview extension functions and updates on .md doc updates. In my opinion, it's well designed and a good pattern set by the vscode dev team other extensions should follow for their doc Preview needs.

I am working on new Data Notebook extension with PRQL and other data tools that I plan to release in April, and will not have the time to spend on these minor UX features and patches, as I find the current prql vscode extension usable in my daily PRQL dev flows and for the extra parts I am integrating in my other data tools.

@richb-hanover
Copy link
Contributor Author

@RandomFractals Thanks for this note. I didn't understand how the new model worked, so I created this ticket. I'm going to close it now, and as you suggest, create a new feature request.

I'll be interested to see your Data Notebook extension. Good luck!

@RandomFractals
Copy link
Collaborator

@richb-hanover great! thanks for opening new ticket for your feature request. I shared my notes on implementing it in #165.

Happy we were able to find a good middle ground solution for your use case.

Btw, I would love to share more info about new Data Notebook ext. with PRQL support , but I did not find any public social media handles you use. Even though I know @max-sixty doesn't like us sharing out of context bits here, you and others can find more info about Data Notebook ext. on twitter, where we shared a few demo gifs, etc.: https://twitter.com/search?f=live&q=(%23DataNotebooks)%20(from%3ATarasNovak)&src=typed_query

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

No branches or pull requests

3 participants