Add a Stdin Specification #586
Conversation
|
@brianmcgee @zimbatm, is there anything I can do to help move this forward? |
|
It's unclear to me if this documents the current behaviour or the desired behaviour. If it's the desired behaviour, isn't there some work needing to be done in |
Yes, treefmt needs changes to support this. I'm happy to implement that (in this PR or a separate one). I just don't want to put in the effort until I know we like this approach. |
|
I'll try to put my brain on this in the next few days. Been a busy few weeks 😫 |
|
@brianmcgee, friendly ping |
|
P.S. Thanks for taking the time to flesh this out, and sorry for taking a while to get around to reviewing this. |
jfly
force-pushed
the
issue-573-stdin-spec
branch
4 times, most recently
from
8e6e1cb to
5671029
Compare
jfly
force-pushed
the
issue-573-stdin-spec
branch
from
5671029 to
d8276a6
Compare
|
@brianmcgee @zimbatm, I could use another review on this. If we're happy with the state of this, then I'd like to work on the underlying implementation. |
| formatting "virtual files" passed via stdin. | ||
|
|
||
| A formatter **MUST** implement the Stdin Specification if its formatting behavior | ||
| can depend on the name of the file being formatted. |
There was a problem hiding this comment.
I agree, but I don't know how to fix it. Does flipping the sentence around help? Suggestions appreciated.
If a formatter's behavior can depend on the name of the file being formatted, then it MUST implement the Stdin Specification.
| "stdin mode". In stdin mode, the formatter reads file contents from stdin | ||
| rather than the filesystem. | ||
| - The formatter _MAY_ understand `--stdin-filepath=<path>` as well, but **MUST** | ||
| understand the space separated variant. |
There was a problem hiding this comment.
It would make sense to outline the purpose of the <path> argument explicitly. Both you and I know why it's there, but maybe not the reader.
There was a problem hiding this comment.
If the path argument is not needed by the tool, it's also valid to ignore it.
There was a problem hiding this comment.
I've taken a stab at this. Please take a look.
| vanilla [Formatter Specification](/reference/formatter-spec), and additionally | ||
| satisfy the following: | ||
|
|
||
| ### 1. `--stdin-filepath` flag |
There was a problem hiding this comment.
| ### 1. `--stdin-filepath` flag | |
| ### 1. `--stdin` flag |
KISS?
There was a problem hiding this comment.
I'm lightly opposed to this, for 2 reasons:
- I think
--stdin-filepathis clearer that the option takes a value. treefmtitself currently implements a--stdinflag with different semantics, and I would like fortreefmtto implement the new stdin spec. Changing this might be awkward?
WDYT?
| Formatters **MAY** also implement the Stdin Specification, which allows | ||
| formatting "virtual files" passed via stdin. |
There was a problem hiding this comment.
How is this support expressed in the treefmt.toml config?
There was a problem hiding this comment.
I was thinking we'd add a new formatter.<mylanguage>.stdin (or stdin_supported) bool that defaults to false if unspecified. Feedback appreciated.
Do you think the stdin spec is the place to put this? I was thinking this would go in our config docs.
This completes step 1 of numtide#573 (comment).
jfly
force-pushed
the
issue-573-stdin-spec
branch
from
d8276a6 to
a94c619
Compare
This completes step 1 of
#573 (comment).