Skip to content

Conversation

@inickt
Copy link

@inickt inickt commented Jan 29, 2026

Enables reading the request json from STDIN when - is specified as an arg instead of a file path. - is commonly used in CLI tools to indicate reading from STDIN.

For example, pbpaste | lk sip inbound create - to use clipboard contents (on macOS) for creating a SIP trunk.

As I write this I realize pbpaste | lk sip inbound create /dev/stdin is functionally equivalent without code changes, so if that is preferred feel free to close...

Summary by CodeRabbit

Release Notes

  • New Features

    • Added support for reading input from stdin by using "-" as an argument, providing an alternative to file paths.
    • Updated help text to document stdin input capability.
  • Bug Fixes

    • Added validation to prevent multiple stdin arguments in a single command.

✏️ Tip: You can customize this high-level summary in your review settings.

@CLAassistant
Copy link

CLAassistant commented Jan 29, 2026

CLA assistant check
All committers have signed the CLA.

@coderabbitai
Copy link

coderabbitai bot commented Jan 29, 2026

📝 Walkthrough

Walkthrough

The changes add stdin support to the request processing logic by treating "-" as a signal to read from standard input. The ReadRequestFileOrLiteral function now checks for "-" before attempting file operations, RequestDesc appends a hint about stdin capability, and createAndPrintReqs validates that "-" is used at most once across multiple arguments.

Changes

Cohort / File(s) Summary
stdin Support Enhancement
cmd/lk/proto.go
Modified ReadRequestFileOrLiteral to read from stdin when "-" is provided; updated RequestDesc with "(or - for stdin)" hint; added validation in createAndPrintReqs to prevent multiple stdin usage when processing multiple arguments.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A dash now opens stdin's door,
No files needed anymore!
Just "-" and watch the data flow,
One stream at once—that's all you need to know! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: adding stdin request support via the '-' argument convention.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

🧹 Recent nitpick comments
cmd/lk/proto.go (1)

72-79: Optional: give a clearer error on empty stdin.
Right now empty stdin surfaces as a JSON parse error; a targeted message can reduce confusion.

♻️ Suggested tweak
	if pathOrLiteral == "-" {
		reqBytes, err = io.ReadAll(os.Stdin)
+		if err == nil && len(reqBytes) == 0 {
+			return nil, errors.New("stdin is empty")
+		}
	} else if _, err = os.Stat(pathOrLiteral); err == nil {
		reqBytes, err = os.ReadFile(pathOrLiteral)
📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4f432a6 and 853e1dc.

📒 Files selected for processing (1)
  • cmd/lk/proto.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: test (windows-latest)
  • GitHub Check: test (ubuntu-latest)
  • GitHub Check: test (macos-latest)
  • GitHub Check: build
🔇 Additional comments (3)
cmd/lk/proto.go (3)

17-22: LGTM: import aligns with new stdin read.


100-103: Nice help-text update for stdin support.


181-191: Good guard against multiple stdin uses.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

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.

2 participants