-
-
Notifications
You must be signed in to change notification settings - Fork 67
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
Render stdin text as Gemtext (#205) #242
Conversation
Signed-off-by: David Jimenez <[email protected]>
Signed-off-by: David Jimenez <[email protected]>
Signed-off-by: David Jimenez <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for creating this PR! Works well for me, just see my comments below. And please fix the linting errors.
stat, _ := os.Stdin.Stat() | ||
return (stat.Mode() & os.ModeCharDevice) != 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this check portable across OSes? Where did you find this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This check leverages the Go standard library so I'd have thought it would be. I haven't tested this outside of my Fedora Linux installation so I wouldn't be able to say for sure though.
I got the idea from here. This article explains the approach in a bit more detail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems good. This answer mentions the same solution working on macOS too. And I guess pipes don't really apply to Windows.
Signed-off-by: David Jimenez <[email protected]>
amfora.go
Outdated
} | ||
|
||
stdinText := stdinTextBuilder.String() | ||
if len(strings.TrimSpace(stdinText)) > 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this check is necessary. Nothing wrong with displaying some spaces or just nothing at all. Would be useful if part of some longer pipeline, where returning nothing is possible but not certain. Much better to get an empty page than be confused by the new tab page.
Other than this I think we're good to go!
Signed-off-by: David Jimenez <[email protected]>
Thanks for adding this! |
This allows
amfora
to accept content via stdin and render it as Gemtext. The main use case this fulfils is the rendering of Gemtext files via stdin (see #205) but plaintext is also handled.