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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
request: add raw paths #497
Conversation
So, my principal concern here is avoiding another I鈥檓 not opposed to this idea, but one thing, is maybe we could take the chance to reformat the struct. Right now, it鈥檚 all clumped together, no paragraphs showing the grouping of fields, and both the There are already unexported fields, so no one could be using the implicit field building of the struct except us, so even reordering fields won鈥檛 break anyone鈥檚 code. Then we can put the four paths together, And we can shorten this up pretty tight: |
Thanks, do you mean reordering the struct like this?
|
will produce the warning |
Ah yes. OK, that鈥檚 why we were doing it that way. OK then, let鈥檚 ignore that improvement. Ideally, if we鈥檙e reimplementing, we would be able to avoid using any locks, the same as |
The lock seems only used to protect the state in close method that could happen concurrently with other requests. It is a read/write lock and in the most of the cases the lock should not be held for write (if my quick look at the code is correct) so it should not be performance critical. I marked this PR as draft because I wanted to hear your opinion on the approach used and wanted to test the implementation of the start directory in a separate sftpgo branch. I'm not sure I have enough time this weekend to do that. So my priority, for now is to test this patch in a real implementation. If it works as expected we can discuss about other library improvements 馃槃 |
After real testing I changed my mind and I would prefer to merge #498 |
This allows to clean the paths externally to the library and allows to handle a start directory for example.
Another option is to add a start directory option to the request server and build the relative path using this directory as base instead of
/
. I would prefer to handle paths myself outside the library but I'm open to discussions. I can also live without this feature 馃槃