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

feat: Trim debug log output #1525

Closed
7 tasks done
taku-nm opened this issue Nov 25, 2023 · 13 comments · Fixed by #1526
Closed
7 tasks done

feat: Trim debug log output #1525

taku-nm opened this issue Nov 25, 2023 · 13 comments · Fixed by #1526
Labels
Feature request Requesting a new feature that's not implemented yet

Comments

@taku-nm
Copy link
Contributor

taku-nm commented Nov 25, 2023

Feature description

I'd suggest trimming any patch that has been included by default from log.
Anything that has been altered by the user should remain included.

Conceivably one might exclude all statements following the regex .* succeeded but this might go against attempts of removing hard-coded variables.

Checklist

  • Trim patches included by default
  • Include note for all patches excluded manually
  • Include note wherever something has been trimmed

Motivation

After a while of seeing feat #1176 deployed, it has become apparent that the Discord message limit has become the main barrier for most users seeking support. The message limit is easily reached by the fact that Apps like YouTube have a lot of patches.
This surely does not affect all possibilities one might reach out for support, but it does hinder the main one.

tl;dr: To remove overall clutter and making it easier to share the logs.

Additional context

Original comment moved from #1176 ;
I'm not entirely sure if this is a feat or bug?
This could be extended to trim even more irrelevant information from the log as well, but isn't really needed.

Acknowledgements

  • This request is not a duplicate of an existing issue.
  • I have chosen an appropriate title.
  • All requested information has been provided properly.
  • The issue is solely related to the ReVanced Manager
@taku-nm taku-nm added the Feature request Requesting a new feature that's not implemented yet label Nov 25, 2023
@oSumAtrIX
Copy link
Member

Every setting or patch option that has not been touched can be trimmed. Exceptions and everything that was touched should be included

@taku-nm
Copy link
Contributor Author

taku-nm commented Nov 25, 2023

Every setting or patch option that has not been touched can be trimmed

Couldn't this result in issues? Hypothetically there could be a patch that does require a value from the user for the patched app to function properly but does not throw an exception when fed the default value.
To notice that scenario the supporter must be aware that the patch needs something else than the default value to function properly, and that the patch is included by default and therefore trimmed.

@oSumAtrIX
Copy link
Member

oSumAtrIX commented Nov 25, 2023

No, if something is not included it is implicitly clear, that it's default

@Ushie
Copy link
Member

Ushie commented Nov 25, 2023

SGTM

@taku-nm
Copy link
Contributor Author

taku-nm commented Nov 25, 2023

Relying on implications within a debug log is not very helpful. This is meant for supporters who may simply have forgotten that some patch requires something else besides default options.
Printing those with options costs nothing and would serve as a helpful reminder.

@oSumAtrIX
Copy link
Member

Nothing that is default has to be included because it serves no purpose. Simply give an example if it were.

@taku-nm
Copy link
Contributor Author

taku-nm commented Nov 25, 2023

What about the reddit patches -> missing client ID?
If the convention is going to be that every patch that requires a value besides default throws ans exception, then this can be left out.

@oSumAtrIX
Copy link
Member

Not possible. Either the user has not entered one (not included in log => its clear) or the user has entered one (its visible in the logs). To begin with, options like that are mandatory. You can not patch Reddit clients with the client spoof patch without entering a client id because it is marked as required. Any valid example?

@taku-nm
Copy link
Contributor Author

taku-nm commented Nov 25, 2023

not included in log => its clear

This is only the case if the supporter knows about this. As I've said above multiple times;

But again,

If the convention is going to be that every patch that requires a value besides default throws ans exception, then this can be left out.

I assume this is what you mean by the second part?

You can not patch Reddit clients with the client spoof patch without entering a client id

If so, Checklist can be adjusted

@oSumAtrIX
Copy link
Member

I don't understand you. The example you give requires the user to enter an option. I asked for an example where not including defaults in the log is a problem

@taku-nm
Copy link
Contributor Author

taku-nm commented Nov 25, 2023

The example you give requires the user to enter an option.

What if the user doesn't do so?

@oSumAtrIX
Copy link
Member

Then they can't proceed to patch.

@taku-nm
Copy link
Contributor Author

taku-nm commented Nov 25, 2023

Then they can't proceed to patch.

Okay so not an issue as I've mentioned above; Will adjust checklist

@Ushie Ushie linked a pull request Nov 26, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature request Requesting a new feature that's not implemented yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants