Site replacement #91
Site replacement #91
Conversation
|
This can be definitely a useful feature with some polishing. First of all, I would make URLs configurable. Also, I wouldn't make it enabled by default. |
|
URLs are configurable in the source code, and it's behind a environment variable to enable it, didn't want to enable it by default :-) |
It means it is active every case by default except you explicitly specify
It isn't configurable, you have to recompile your code every time you'd want to change it. By configurable I mean a config file for example. |
Damn that is embarrassing, sorry, I wrote the code last night a bit too early into the morning. Oh dear, sorry about that. The logic never clicked in my head. Just assumed DEBUG would be disabled by default. My bad, alright. Any specific format you'd want for the configurable URLs? Also are you on Matrix? Tried to look for you in the searx channels but couldn't find you. I think Morty is an awesome project, and I'm hoping to contribute some more changes I've been running on my instance and playing about with. |
No need to apologize, we all make mistakes, that's why code review is useful. =)
It would be good to use an easily editable user-readable format, what about JSON or a simple CSV?
I'm online both on freenode's and ircnet's
Awesome! |
|
Added json config support and swapped to using flags rather than environment variables. :-) |
the0d0re9
force-pushed
the
site-swap
branch
4 times, most recently
from
a5a9b90 to
af8f4e1
Compare
|
Sorry for the force pushes, git is not kind to mistakes :-( |
morty.go
Outdated
| @@ -991,6 +1004,7 @@ func main() { | |||
| version := flag.Bool("version", false, "Show version") | |||
| requestTimeout := flag.Uint("timeout", 2, "Request timeout") | |||
| socks5 := flag.String("socks5", "", "SOCKS5 proxy") | |||
| siteReplace := flag.Bool("replace", false, "Enable experimental site redirection") | |||
There was a problem hiding this comment.
Instead of a bool switch this could be used to set the path of the json file.
There was a problem hiding this comment.
Also, I'd remove the "experimental" word.
There was a problem hiding this comment.
If that's the way you want to go we can do that, but I feel like using that as a path to the json file lends itself to being enabled by default. If we set a default people will automatically use it, if we don't nobody will know about it. It's your decision though.
There was a problem hiding this comment.
Don't you think that will cause more people not to use it? I really think that's adding complexity, it's not a well known feature, how will they know sites.json is the site replacement file
|
Another consideration to make, should we modify the banner to say some sites have been remapped? Obviously most of the people operating morty wouldn't abuse this maliciously, but it may be worth adding just for clarity to users. |
This is definitely a good idea |
Any preference on where this message is? or how we show the user that? |
Probably the best would be to display it on the currently viewed page if there are rewrites. What do you think? |
Works for me, this will require quite a bit of a refactor as the current layout of the codebase isn't super suitable for that sort of thing, is that alright? |
|
I have some local changes, let me push it, probably it will make the implementation easier. |
|
Pushed, take a look at the new body extension, it uses go templates, which probably makes it easier to add new values to it. |
|
That does make it much easier, thank you :-) |
|
No problem. The new layout can be buggy with sites using fancy css, so it needs a bit more refinement, but we can do it later. I tested it with quite a few sites and it works well so far. |
Still tracking this, just had some personal issues that mean I've been unable to code for the last few days |
Thanks, no worries, take your time. =] |
|
@the0d0re9 so how is it going? no pressure, just a friendly reminder this existed :) |
Implementation for #90