Allow optional ignoring of SSL, display What's New in details and clarify flags

[tyr7z]

I took this idea from cURL. It also has this flag.

[NicknineTheEagle]

What is the purpose of this feature?

[tyr7z]

Its purpose is to ignore or skip the SSL pinning from net/http, so that a proxy can be used to decrypt and therefore debug the HTTPS requests. I found it useful, and because cURL provides this flag, I thought it would make sense to add it here as well since this is also a tool to "send API requests".

[tyr7z]

Also, I wanted to ask: do you have Discord, Telegram, or any other messaging app where I could contact you?

[tyr7z]

I just added another small commit—hope it looks good.
Should I rename the pull request and/or force-push to combine the two commits into one?

[Yakov5776]

The rest of the changes seem to be from my fork

[NicknineTheEagle]

The rest of the changes seem to be from my fork

@tyr7z Yes, it looks like you've pushed more than you had meant to.

[tyr7z]

Yes, the pull request was originally meant for just the -k flag but I figured adding these too wouldn't hurt.
Do these changes look fine to you? Or do you see any issues with them. They're just some little improvements but I'm up to suggestions.

[NicknineTheEagle]

Yes, the pull request was originally meant for just the -k flag but I figured adding these too wouldn't hurt. Do these changes look fine to you? Or do you see any issues with them. They're just some little improvements but I'm up to suggestions.

First of all, keep PRs focused, please. Secondly, I don't see a point in merging these - it's mostly fluff changes that, if I had to guess, @Yakov5776 probably made for personal convenience, otherwise they would have opened a PR themselves.

[tyr7z]

First of all, keep PRs focused, please.

Sorry, I just changed the PR's title so it's more clear. I should have done that before or have made the PR as it is now right from the start.

I don't see a point in merging these.

They're features at the end of the day, especially the SSL ignoring and the What's New information.

It's mostly fluff changes that, if I had to guess, @Yakov5776 probably made for personal convenience.

Of course they're for convenience, but isn't the whole tool for personal convenience too?

@Yakov5776 does want these changes added. I just decided to get them on this pull request because they're so small and this way it's clear that I show support for them (at least that's what crossed my mind). That's the reason why I added him as a co-author on the commit.
I saw no point in having multiple pull requests for 33 lines changed.

As I said, I took the idea of the -k flag from cURL. It's useful if you want to proxy the requests (because it makes SSL pinning of net/http ineffective), which I wanted to do to understand the API easily without having to go through Golang code, which I'm personally not proficient at.

I think it doesn't need an explanation, but the What's New information on the details option makes it more complete and it's sometimes useful to have when the developers of the app you're targeting actually take the effort to put something descriptive on it.

[NicknineTheEagle]

They're features at the end of the day, especially the SSL ignoring and the What's New information.

Doesn't mean that every possible feature should be added. Scope matters.

Of course they're for convenience, but isn't the whole tool for personal convenience too?

The point was about personal vs broad use but if Yakov wants these added then never mind.

I saw no point in having multiple pull requests for 33 lines changed.

Once again, organization. I'd rather not have a bunch of unrelated features dumped into one PR. PRs can be however big you want them to be, it's not like GitHub has a hard limit on that.

Anyway, once again, I have no desire of merging any of this except the initial SSL change which does seem somewhat useful and .editorconfig which is a good addition. I see no point of renaming -d to -c and I'd rather not change parameters unless really necessary (like we had to do with -download). I thought about adding What's New to details output before but the problem is that it appears as raw HTML code and it's often quite long which makes details output a cluttered mess. The rest of the code is just minor refactoring.

[tyr7z]

Doesn't mean that every possible feature should be added. Scope matters.

That's right. Although I do believe these changes should be added because they're on the scope.

I'd rather not have a bunch of unrelated features dumped into one PR.

Agreed. That was completely my bad. This is my second pull request ever, so I'm still learning. Sorry about that.

Anyway, once again, I have no desire of merging any of this except the initial SSL change which does seem somewhat useful and .editorconfig which is a good addition.

I'm glad you found those helpful at least. They definitely were for me, however I wasn't able to make VSCodium use spaces instead of tabs through .editorconfig for the formatter. I have to tinker more with it until something does the job.

I see no point of renaming -d to -c and I'd rather not change parameters unless really necessary (like we had to do with -download).

I understand your point of wanting to keep parameters unchanged. It's a pain in the ass for example with frida when you use different versions and some flags or functionality turns out to be different across them. Also when you make a script that uses some CLI app (like this one) by running commands, you want it to be stable. Or legitimately just because it's annoying to change the flags when you're used to typing a set of them. I've been there.

On second thought, it does make sense for "checkin and sync device" to be -d. Because it does not do "checkin" alone but the two device operations in a single command.
There are some starting letter collisions so I think there's no perfect way of making all of these short. Which would make it more consistent, in my opinion.
It currently would be like so:
-a: application ID
-acquire
-asset: download asset module
-bulkdetails: /fdfe/bulkDetails request
-d: checkin and sync device
-download
-k: skip SSL validation (my new flag)
-l: languages
-o: oauth_token
-p: platform
-s: single APK
-v: version code

So for example why not change -a (app) to -i (ID). And then -acquire to -a. If I recall correctly, it was like this on the original repository. But honestly, it feels nicer to keep -a as app. Or it could also be -app and -a for acquire...

I guess they're fine as they are. However I just got an idea.
-acquire and -download are pretty much the main flags. They're often used one after the other but yet they're the longest ones. So why not create a new -g (stands for get) flag that does acquiring and downloading all in one?

It seems like a good alternative to changing the existing flags to me. What do you think?

I thought about adding What's New to details output before but the problem is that it appears as raw HTML code and it's often quite long which makes details output a cluttered mess.

Fair enough. Guess we'll keep that in my fork or Yakov's.

The rest of the code is just minor refactoring.

So, just to be sure, in summary: I revert -c flag to -d and get rid of What's New?
Can I just comment "What's New" out on details.go's String() function?

Sorry for making my messages so long.

Also, may I ask once again. (Not sure if you ignored or just forgot to reply earlier). Is there some messaging app that I can reach you on?

[NicknineTheEagle]

It seems like a good alternative to changing the existing flags to me. What do you think?

Hmm, let's just save this discussion for later.

So, just to be sure, in summary: I revert -c flag to -d and get rid of What's New?

Pretty much, yeah, just revert this to your initial PR with SSL parameter + editorconfig, that would keep it concise enough.

Can I just comment "What's New" out on details.go's String() function?

Eh, fair enough, it's good information to keep. You could also add a note as to why it's commented out. Not sure if it's worth keeping in this PR or if it should be split into a separate one, though.

Also, may I ask once again. (Not sure if you ignored or just forgot to reply earlier). Is there some messaging app that I can reach you on?

I guess you could join Video Game Preservation Collective server, I post about my Android and Google Play research there: https://discord.gg/exTsZfg2

[tyr7z]

Hmm, let's just save this discussion for later.

Alright.

Pretty much, yeah, just revert this to your initial PR with SSL parameter + editorconfig, that would keep it concise enough.

Eh, fair enough, it's good information to keep. You could also add a note as to why it's commented out.

I just amended the commit and force-pushed.
Changed it back to -d and commented out the What's New with a link to this pull request and a very brief explanation of why it's disabled.

I guess you could join Video Game Preservation Collective server, I post about my Android and Google Play research there: https://discord.gg/exTsZfg2

I just joined and posted an introduction.
I'm tyr7z on Discord too, if you want to check it out.

I believe this pull request should be ready to merge but don't hesitate to ask for more changes if needed.

[NicknineTheEagle]

You have four different changes here:

• Added .editorconfig
• Added a parameter to skip SSL verification
• Added What's New printing to details output (commented out)
• Updated README

Split them into separate commits for organization and clarity.

[NicknineTheEagle]

More clarity, please. Explain what a "single APK" means to the end user.

[tyr7z]

Okay. Reviewed changes applied and commits split.
Tell me if you want me to edit the README again. I think there's still margin to improve the single APKs' explanation.

[NicknineTheEagle]

What's this about?

[tyr7z]

It's nothing. I just used WinMerge to compare some files and it creates those .bak when you do edits. I guess it doesn't hurt to have it on the gitignore.

[NicknineTheEagle]

How come just .bak isn't good enough? This .gitignore is a standard one that came from GitHub.