-
-
Notifications
You must be signed in to change notification settings - Fork 242
feat: Match fingerprints by instruction filters #329
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
base: dev
Are you sure you want to change the base?
Changes from all commits
31f6122
1965e84
d02aad0
a160101
7547319
14335e8
3386fd9
e2707e1
9779e50
4d38837
a3852a6
2b6e437
319a8a7
1199e21
111d6ca
3dad1b0
6c80a20
cfb873a
c37ecb8
231378e
502ea98
f5a1b26
1166096
cdb986d
0e85451
b3b77ac
47e8086
0ca165d
f54efb1
a572771
2faba7f
b74d51b
df7bc88
2c91090
a0a0306
329dfbd
b117dba
bb06381
e1930ea
f981d1c
20b4900
132fa00
f93f870
955ceb6
c74c1b8
5885984
57d8087
1dacd3d
b08ef19
0f198c4
142aa71
8951990
772c0e6
8f7911e
567feef
d7974ef
d0e0a80
44a1424
fc319f4
6c58248
6898576
e4bfbce
d1d5557
5628204
f32cc90
f38f5a7
22db561
f675f86
6cc7411
74b54d2
3f6bea1
8ceea4c
ebafb4c
93535e9
4f02909
3ace863
c0e6eae
c5b0b42
9fc7237
b0c31b5
6e72e32
f266622
a2a0b5b
dfbea7e
254e8d6
da11336
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -85,7 +85,7 @@ val disableAdsPatch = bytecodePatch( | |
// Business logic of the patch to disable ads in the app. | ||
execute { | ||
// Fingerprint to find the method to patch. | ||
val showAdsFingerprint = fingerprint { | ||
val showAdsFingerprint by fingerprint { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The by API is undocumented anywhere. I had to lookup the source code to understand its existence or difference and when to use = or by. How does the new API in this PR behave differently from current dev? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The 'by' semantics allows capturing the fingerprint name ( This was briefly discussed in one of the other conversations in this PR. Without this PR, it's more difficult to tell which fingerprint failed to resolve. And for some code it's impossible to disambiguate which fingerprint failed such as looping over multiple fingerprints:
And with this change the problem is immediately clear:
|
||
// More about fingerprints on the next page of the documentation. | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the reason for naming the files like this? If they are renamed, they should be named like this in the other repos where we have the same kind of docs as well for consistency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I renamed them so the files are sorted by number, and they show in the same order as presented.
https://github.com/ReVanced/revanced-patcher/blob/main/docs/2_patches_intro.md
Notice on the left file selector, 2 comes after 2_1, which is confusing when navigating thru the docs.
But if they're renamed, the file sorting is correct and matches the naviation:
https://github.com/ReVanced/revanced-patcher/blob/f675f865bc435dcd149a8602582a12785190a385/docs/2_0_0_patches_intro.md
The other repos docs could be renamed if any have the same inconsistent file ordering compared it's navigation order.