listhen icon indicating copy to clipboard operation
listhen copied to clipboard

feat: unix domain sockets and windows named pipe support

Open Mastercuber opened this issue 2 years ago โ€ข 8 comments

๐Ÿ”— Linked issue

Closes #1

โ“ Type of change

  • [ ] ๐Ÿ“– Documentation (updates to the documentation, readme, or JSdoc annotations)
  • [ ] ๐Ÿž Bug fix (a non-breaking change that fixes an issue)
  • [x] ๐Ÿ‘Œ Enhancement (improving an existing functionality like performance)
  • [ ] โœจ New feature (a non-breaking change that adds functionality)
  • [ ] ๐Ÿงน Chore (updates to the build process or auxiliary tools and libraries)
  • [ ] โš ๏ธ Breaking change (fix or feature that would cause existing functionality to change)

๐Ÿ“š Description

Here is yet another hopefully promising PR :grin:

These PR adds support for listening on unix domain sockets or on windows named pipes. It adds the assignable CLI options, copies the full socket/pipe path to clipboard if wanted, prints the socket path and ignores all HTTPS options and also the --open option.

While developing this PR, I recognized, that the close() function is neither called when the terminal is closed, nor when the process gets interrupted with Ctrl + C, so I added 3 signal traps to trigger the close call. With these traps the socket will get automatically cleaned up; without these traps - and without proper termination - the socket needs to be deleted manually before starting listhen again.

2 tests are included. Unlike the h3 app test, the handle test is returning the same result for non existing paths. Is this expected behavior?

I've successfully executed the tests on Manjaro and on Windows 10.

๐Ÿ“ Checklist

  • [x] I have linked an issue or discussion.
  • [x] I have updated the documentation accordingly.

Mastercuber avatar Aug 14 '23 00:08 Mastercuber

Codecov Report

Attention: Patch coverage is 59.25926% with 22 lines in your changes are missing coverage. Please review.

Project coverage is 50.57%. Comparing base (d115045) to head (0b06e49). Report is 14 commits behind head on main.

Files Patch % Lines
src/listen.ts 64.51% 11 Missing :warning:
src/cli.ts 0.00% 6 Missing :warning:
src/_utils.ts 61.53% 5 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #104      +/-   ##
==========================================
+ Coverage   49.42%   50.57%   +1.14%     
==========================================
  Files          17       17              
  Lines        1819     1835      +16     
  Branches      147      156       +9     
==========================================
+ Hits          899      928      +29     
+ Misses        915      902      -13     
  Partials        5        5              

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Aug 14 '23 00:08 codecov[bot]

Thanks for your review BTW!

Mastercuber avatar Aug 14 '23 18:08 Mastercuber

@pi0 After a bit of refactoring, this PR looks much cleaner now.

Adding the signals traps is separated into #108 and an issue #107 was created for it.

The getURLs function now uses getURL also for sockets and just returns earlier.

Assertions used in both tests (http and https version) are extracted into functions.

Also the docs looks a bit cleaner now. With --ipc [name] and Default: false [listhen] it should be quickly possible to understand what is meant with it.

Mastercuber avatar Aug 16 '23 21:08 Mastercuber

Thanks for putting efforts on this PR. It looks nice. I have added few refactors to simplify impl. Probably need more time to resolve merge conflicts and add little more fixes (tests are broken on macOS as it uses a random temp dir also we shall support absolute paths for socket that are outside of /tmp).

Feel free to rebase or make any of those changes if had time to spend and have a nice weekend!

pi0 avatar Sep 08 '23 23:09 pi0

Indeed the generator wasn't necessary for passing the args.. I somehow have overseen the possibility to pass an object to server.listen.

Absolute paths on unixoid systems are also supported now and full pipe paths. Actually supporting full pipe paths seems unnecessary since they have to start with \\?\pipe\<pipe-name> and are therefore no actual system paths.

Also have a nice weekend :beach_umbrella:

Mastercuber avatar Sep 09 '23 23:09 Mastercuber

Thanks again for all your hard work @Mastercuber, IMHO this PR looks really good now :-) would be great to merge it so we can have UNIX socket support for Nuxt devserver!

Cheaterman avatar Oct 17 '23 10:10 Cheaterman