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.

Sign up for GitHub

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

Jump to bottom

Update; Add read parsing options #5

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open

Update; Add read parsing options #5

wants to merge 16 commits into from
+253 −134

Conversation

jsejcksn
Copy link

@jsejcksn jsejcksn commented Jun 11, 2020 •
edited
Loading

This PR resolves #3 and resolves #4.

Notes

It is technically a re-write (I wrote much of the code before discovering your repo, but found it when I encountered a Powershell issue). Instead of publishing a new Deno module, I have reviewed your code, and restructured the code in this PR to more closely align to the existing structure.

The PR focuses on addressing a few core issues:

  • Updating the code to work with Deno's current API
  • Adding read parsing options for newline control characters and trailing newlines
  • Supporting both xsel and xclip on Linux (in that order)
  • Adding support for Linux on WSL

An important note: the export format is changed, so the major version should be increased if published.

Previously, the methods could only be accessed on the clipboard export:

import {clipboard} from 'https://deno.land/x/clipboard/mod.ts';

await clipboard.writeText('hello world');
await clipboard.readText();

Now, there is more flexibility for importing.

Like this:

import * as clipboard from 'https://deno.land/x/clipboard/mod.ts';

await clipboard.writeText('hello world');
await clipboard.readText();

Or like this:

import {readText, writeText} from 'https://deno.land/x/clipboard/mod.ts';

await writeText('hello world');
await readText();

Thanks for writing this module. Please review and let me know your thoughts. Cheers!

@jsejcksn
Add VS Code workspace setting for Deno extension
d77e294
@jsejcksn
Remove unnecessary "net" permission
a63d688
@jsejcksn
Update and refactor for future development
5ef81f0
@jsejcksn
Refactor; Add tests for readText options
256e01f
@jsejcksn
Update imports
2b17682
@jsejcksn
Update for refactor
f64f50e
@jsejcksn
Update Deno install version to 1.0.5 for Linux/Mac
f928322 
and remove unnecessary "net" permission for tests
@jsejcksn
Add explicit clipboards type
66505d8
@jsejcksn
Use automatic var instead of env var for Windows
85740a0 
See https://docs.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_automatic_variables?view=powershell-7#input
@jsejcksn
Add explicit types for exports; Add JSDoc comments
50e65d8 
for https://doc.deno.land

Related isssue at denoland/doc_website#113
@jsejcksn
Inline run options
b3c32fa 
to address type narrowing issue in Deno v1.1.0: denoland/deno#6270
@jsejcksn
Replace "trim" option with "trimFinalNewlines"
cbfbbfe 
and catch exceptions while determining Linux binary
@jsejcksn
Update readme to reflect updatd read options
762d665
@jsejcksn
Update read options in tests
5f8f7f4
@Nisgrak
Copy link

Nisgrak commented Jun 28, 2020

Doesn't work in wsl. It gives this error:

xsel: Can't open display: (null)
: Inappropriate ioctl for device
error: Uncaught Error: There was a problem writing to the clipboard
    if (!success) throw new Error(errMsg.genericWrite);

@jsejcksn
Copy link
Author

@Nisgrak That means that your environment was not able to be detected as WSL.

Can you provide some information about your environment?

  1. Is it WSL or WSL 2?

  2. Which terminal are you using?

  3. Which distribution have you installed from the Windows store?

  4. Will you please copy and paste the output of the following three commands? They are currently what is being used to make the determination:

uname -r -v
which clip.exe
which powershell.exe

@Nisgrak
Copy link

Nisgrak commented Jun 28, 2020

@jsejcksn

  1. WSL 2
  2. Fluent Terminal
  3. Ubuntu

image

@jsejcksn
Copy link
Author

@Nisgrak I’lll look into this more. Those executables with the .exe extensions should be in your path. See https://docs.microsoft.com/en-us/windows/wsl/interop#run-windows-tools-from-linux

Have you modified your path or installed tools which might have modified your path?

@jsejcksn
Copy link
Author 

uname -r -v
which clip.exe
which powershell.exe

@Nisgrak Will you try those commands using a first-party Microsoft terminal application like %windir%\system32\cmd.exe or Windows Terminal?

@Nisgrak
Copy link

Nisgrak commented Jun 29, 2020

I fix the problem with the .exe, windows didn't modify my PATH so I added the routes by hand.

I try again with the lib and now works! Looks like the problem was the routes in PATH...

Anyway, in cmd:
image

@jsejcksn
Copy link
Author

@Nisgrak That's great to hear. So everything is working now: the tests pass when you run deno test --allow-run in the repo directory?

@Nisgrak
Copy link

Nisgrak commented Jun 29, 2020

Yes! Pass the 8 test in WSL and Powershell like a charm 😄.
Good job!

@jsejcksn
Copy link
Author

@rsp Thoughts?

@Nisgrak
Copy link

Nisgrak commented Jun 29, 2020

Maybe you should add a warning or troubleshoot in the readme about that error, or some concrete error if in you kernel version contains "microsoft" and haven't access to .exes.

@jsejcksn
Copy link
Author

Maybe you should add a warning or troubleshoot in the readme about that error, or some concrete error if in you kernel version contains "microsoft" and haven't access to .exes.

@Nisgrak Added in 4f7dfd2

Repository owner deleted a comment from hyassine1 Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update for deno v1.0.0
2 participants
@jsejcksn @Nisgrak