Skip to content

Conversation

@danp
Copy link
Contributor

@danpdanp commented Nov 14, 2024

Description

Updates #192

Adds \pipe_once / \| command for sending output to a program. Mostly copied from the same in mycli.

I also added a test for .once as part of getting started and changed it slightly to be structured more like mycli's version.

Checklist

  • I've added this contribution to the CHANGELOG.md file.

else:
with tempfile.NamedTemporaryFile(delete=False) as f:
litecli.packages.special.execute(None, "\\pipe_once tee " + f.name)
litecli.packages.special.write_pipe_once("hello world")
Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

I changed this up from mycli, using tee to write the output to a file which can then be checked. But I did notice around here that mycli is using write_once in this test instead of write_pipe_once.

@danpdanp mentioned this pull request Nov 14, 2024
litecli.packages.special.write_pipe_once("hello world")
litecli.packages.special.unset_pipe_once_if_written()
else:
with tempfile.NamedTemporaryFile(delete=False) as f:
Copy link
Member

Choose a reason for hiding this comment

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

Curious why the delete=False here?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Must have copied from above or elsewhere. But looks like if we know we aren't on windows we don't need this. Removed!

@amjith
Copy link
Member

Apart from the one minor comment in the test, this looks good to me.

@amjith
Copy link
Member

Thank you for taking the time to do this. I appreciate it very much. Thanks for pointing out the oversight in mycli test. I'll fix that as well. 😄

Mostly copied from mycli. Add tests for once as part of getting that going. Updates #192
@amjithamjith merged commit 233d84b into dbcli:mainNov 23, 2024
7 checks passed
Sign up for freeto 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

@danp@amjith