Portability patch #512
Portability patch #512
Conversation
There was a problem hiding this comment.
Thanks for the PR 😊
To be honest, I'm a bit hesitant to merge this as it is right now, mainly because the code is very specific to your project and it's branding. It's seems like bad practice to me, especially with regards to Separation of Concerns.
That said, I believe we can fulfill your specific requirements while still maintaining good SoC between the two projects - we just need to make some small adjustments to a few packages in Pat.
Please see the inline comments regarding setting custom HTTP listen addr and data dirs.
The way I see it, you have 4 main requirements:
- Set custom HTTP listen addr
- Set custom config/state/log dirs
- Get the version string
- Load the embedded fs
I believe the last two is the only remaining requirements that currently requires you to access internal packages/files. It is already possible to import github.com/la5nta/pat/app and other non-internal packages.
I think we can expose the version information through a proxy method in app package. And as for the embedded fs: I think that can be achieved by having a web/embedded_fs.go file and let the api package import that new package web directly.
What do you think about this? Is there anything else that I might have missed?
Thanks 😊
| flagRandomisePort = rndport | ||
| } | ||
|
|
||
| func doRandomisePort() error { |
There was a problem hiding this comment.
Instead of setting the custom port by rewriting the config file, you have two other options:
- Pass
-addr [host:port]as arguments to thehttpcommand. - Set env var
PAT_HTTPADDR=[host:port](this applies to all config options)
By doing this, you don't need to read/write the config file.
| @@ -0,0 +1,86 @@ | |||
| package porta_paths | |||
There was a problem hiding this comment.
Setting custom paths can be done with standard XDG environment variables (XDG_DATA_HOME, XDG_CONFIG_HOME and XDG_STATE_HOME).
This should eliminate the need for the porta_paths package I believe?
|
This is just a suggestion, feel free to change anything you like. Regarding the version string - this function can be removed, i forgot to. How do you see bringing Android support into the mainline? Is it very low priority and should stay as a separate project for now, or should i remove all the "different project" branding? |
I'm not sure... I think maybe having it as a separate repo would make sense, or possibly a I don't think we should make it the highest priority right now, but I'm motivated to spend some time on it :) If it's ok with you, maybe we can continue with portapat (and libportapat) as an experimental side project with the goal to eventually move it into the main repo or as a side repo under the same organization? If so, I need to kindly request that you relicense it under MIT. We can't merge code that is copied from a GPL licensed repo, and relicensing down the road might be a challenge if you get contributions from other people aswell. |
The embedded filesystem was in package main, making it inaccessible to alternative main packages that need to serve the HTTP interface. Moving the embed directive and web handlers to a dedicated web package allows any package to import and use the web UI functionality. This also improves separation of concerns by isolating static file serving, template handling, and dev server proxying in a single package, and simplifies the API by removing the staticContent parameter from NewHandler. Ref #512 and #15
|
With commit 946a91f I believe all four requirements are met, so we should now support having a android specific main package outside this repo that is able to start the web gui. I have also fixed the missing "Back to mailbox" button in the Configuration page on small screens. Haven't rebuilt the web assets yet, but will do so before the next release 😊 |
|
Ok, as time permits I'll relicense and megre/change/rerequest accordingly. |
|
Thank you! Looking forward to it 😊 Please let me know how the recent changes on |
2 participants
This is a proposal to add "portability library" build mode to PAT, which paves the way for Android support.
A few notes:
So, let me know what you think, what should be improved or changed.