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

Guidelines for implementing LynxChan support #362

Closed
TheStranjer opened this issue Sep 25, 2019 · 12 comments
Closed

Guidelines for implementing LynxChan support #362

TheStranjer opened this issue Sep 25, 2019 · 12 comments
Labels
new site request A request to support a new chan question Further information is requested

Comments

@TheStranjer
Copy link

I'm considering implementing LynxChan support, and not just for an enumerated list of LynxChan instances, but one where it detects if a given domain name is a LynxChan instance and goes from there.

Since this would be a fairly major addition, I don't want to implement something and then have the pull request get rejected summarily because of some autism. I also do not have the option to really discuss this anywhere since #361 explicitly rejects the use of IRC/Discord for doing so.

So, what lines should I draw within such that I can reasonably expect a pull request to this effect to get accepted? Be specific.

@Adamantcheese
Copy link
Owner

I can't think of anything that would cause me to outright reject anything; all of the code that would be determining whether or not a site is running Lynxchan would probably be under CommonSiteURLHandler methods or something like that. Basically just submit a PR and you'll be fine. I mean, K1rak submitted a PR with that beast that was thread downloading and I just kinda went with it.

As long as it doesn't break any other sites, you're good.

@Adamantcheese Adamantcheese added the question Further information is requested label Sep 25, 2019
@TheStranjer
Copy link
Author

Thanks! I naturally would never make it break other sites. I'd make new classes for LynxChan stuff. In fact, I wrote the thing that detects it -- but doesn't implement it yet -- in a completely different fashion. The reason I was hoping for an IRC or a Discord is so that this sort of topic could be dealt with rather casually; I didn't know CommonSiteUrlHandler existed. Instead, I did something completely different. You can see my commit on my fork here.

Naturally, writing code that conforms to the established approach of the software is superior to writing code that does not. So I think I will sleep on this and rewrite what I have later. Thanks for the tip.

@Adamantcheese
Copy link
Owner

The only issue I see with your implementation is one of asynchronosity; that is, when adding the site, you should immediately wait for a result, otherwise the use can spam that request.

You may also run into issues with DatabaseSiteManager and SiteRegistry if you're doing things dynamically, which is one reason why I'd actually avoid it, because I can tell it's going to be a goddamn nightmare trying to get it to work with other features (notably thread saving and settings import/export) more than likely.

@TheStranjer
Copy link
Author

TheStranjer commented Sep 25, 2019

Yeah but I think that being able to dynamically detect LynxChan instances (or instances of other engines in the future) is actually pretty key because, philosophically, you want to be able to just throw any random site in there. Maintaining enumerated lists of pre-accepted websites to me seems like folly. (I have some other ideas for boardwide watching, allowing for many rinky dink chansites to become more viable, but this is a tangent for another thread.)

One way to avoid spamming requests is to disable and then re-enable the button when they try to go for it, so that they can still "spam" it successively, but not spam like five times because it took half a second too long for their liking for the website to respond.

As for doing it in CommonSiteUrlHandler, you might be able to make respondsTo synchronously determine if it's a LynxChan instance with an HTTP request, which is my initial thought on the matter. However, this could run into the issue of making the whole app lag out for several seconds with an ANR.

All-in-all, this is something for me to sleep on.

@TheStranjer
Copy link
Author

@Adamantcheese I implemented a technique to detect LynxChan instances. The commit is here. What I will implement next is actually adding the site. I'm going to be implementing things in the following order:

  • add instance,
  • add board,
  • read board,
  • read thread,
  • reply to thread,
  • upload files,
  • post new threads

I'm hoping for you to glance at my implementation for these as I write them, to ensure that I'm not doing something really stupid, and not building a house of cards that will ultimately be rejected. Is this reasonable to you?

Thanks for taking the time to read this.

@Adamantcheese
Copy link
Owner

Adamantcheese commented Dec 20, 2019

I'm going to be away until the new year, so I won't be able to give super detailed comments. K1rak might be able to provide some guidance as well, but whenever you submit a PR then I'll definitely do a thorough review. Just do what you can to make it work for now.

@K1rakishou K1rakishou added the new site request A request to support a new chan label Jan 6, 2020
@fch1980
Copy link

fch1980 commented Jan 30, 2020

Are you still on this TheStranjer ?
This would be an awesome feature.

@K1rakishou
Copy link

@TheStranjer Hi, I just looked through the PR and noticed that you are using Volley for network requests. The thing is we are planning to get rid of Volley, eventually, in favor of OkHttp (since it's kinda industry standard and we already use it in the rest of the project), so it would be really nice if you could use OkHttp instead of Volley too.

@TheStranjer
Copy link
Author

Unfortunately, I decided to not continue this because StephenLynx made a point to state that he has no intention of working with me on it. Basically, he regards API access to his system wholly unnecessary and that people should simply use the web app. Because of some design issues with the way he structures his API responses, and his reluctance to take seriously how that could impact an Android app's usage of his API, I've decided to move on from this project. I'm very sorry for wasting you guys' time.

@Adamantcheese
Copy link
Owner

Tis' sad to hear, but that's just how it goes with *chans.

@StephenLynx
Copy link

You are full of shit, @TheStranjer . You keep making up shit that I never said. I said that MY solution was adding mobile support to the front-end. The engine ALREADY have a complete api to be used, including with the intention to be used on cases like this. Specially when it comes to responses. Every single fucking page and action can be returned as json. And there are no "design issues", you are just nit picking. What "impact" is this? You not having the sitle's <title>? Come the fuck on.

@StephenLynx
Copy link

And btw, how far did you ACTUALLY worked? Did you really try the api and using it or just assumed it would fail and did fuck all, then blamed me? Just like when you ASSUMED floens wouldn't accept lynxchan support unless it had an "engine" field on index.json and never did anything with that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new site request A request to support a new chan question Further information is requested
Projects
None yet
Development

No branches or pull requests

5 participants