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.

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

Enhancement: In REPL mode read bytes from file #279

Closed
BenSlabbert opened this issue Apr 30, 2020 · 6 comments
Closed

Enhancement: In REPL mode read bytes from file #279

BenSlabbert opened this issue Apr 30, 2020 · 6 comments

Comments

@BenSlabbert
Copy link
Contributor

BenSlabbert commented Apr 30, 2020

Thanks for an awesome tool :)

While working on a new feature I needed to be able to send binary data. The existing support using JSON and base64 encoding is great, however I feel we can improve the REPL with the following change:

When prompted for TYPE_BYTES allow the user to specify a path to a file which is then read in.

For example:

data (TYPE_BYTES) => file:///full/path/to/file.txt

I have also created a pull request: #280 with this suggested change.

@ktr0731
Copy link
Owner

ktr0731 commented May 2, 2020

Thanks for proposing a new feature.
I'm sure it's a great feature that will greatly enhance Evans more user-friendly!

I want to suggest some points about implementation.
First, "file" URI scheme defined in RFC 8089 doesn't allow relative path, so this testcase must read from /proto.go, not ./proto.go.
Also, we assume that introduce the feature with the strict "file" URI scheme, it's inconvenient because we cannot specify files by using relative path representation.

So, how about specifying files by using path/filepath representation just like ./proto.go? I think it's straightforward usage to almost users.
However, by introducing this change, it's not able to specify a file path as "string". So, we should implement some additional things such that disable file path interpretation (maybe a flag?).

@BenSlabbert
Copy link
Contributor Author

Great suggestions thank you!

I agree using a relative filepath will be better instead of the full path in terms of usability.

with regards to:

However, by introducing this change, it's not able to specify a file path as "string". So, we should implement some additional things such that disable file path interpretation (maybe a flag?).

should we add a flag that will by default interpret the input to TYPE_BYTES as a relative file path?
is this for each REPL session or for each grpc service call?

@ktr0731
Copy link
Owner

ktr0731 commented May 6, 2020

should we add a flag that will by default interpret the input to TYPE_BYTES as a relative file path?

We should, it's better than adding a flag that disables flag interpretation 👍

I think it's better to define a flag that belongs to the REPL call flag set to narrow down the influence of the flag. If there is an inconvenience, it can be redefined in a larger scope.

@BenSlabbert
Copy link
Contributor Author

I will update the PR accordingly :)

@BenSlabbert
Copy link
Contributor Author

PR updated :)

@ktr0731
Copy link
Owner

ktr0731 commented May 24, 2020

Merged in #280

@ktr0731 ktr0731 closed this as completed May 24, 2020
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

No branches or pull requests

2 participants