-
-
Notifications
You must be signed in to change notification settings - Fork 147
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
Handle error while creating app directory #442
base: v2
Are you sure you want to change the base?
Conversation
Your PR is completely breaking the code style of the project. I know eslint is badly configured and doesn't cover this folder, but when contributing to a project you really shouldn't mess around with the formatting (even if the editor does it "automatically"). |
@cyyynthia I actually didn't even notice that when I first made this PR (as you guessed, my editor did it). It's fixed now. |
"I found that the problem was a discrepancy in the way that this specific package installed Discord Canary." As the packager: this is so it uses the system electron, instead of the bundled electron by discord (which works like ass on linux). There must be a way to make Powercord detect it instead of making it so it crashes (and I think it detects the sibling, discord_arch_electron) |
there is indeed a very simple way to make it work: diff --git a/injectors/linux.js b/injectors/linux.js
index d3086a4..68b4e60 100644
--- a/injectors/linux.js
+++ b/injectors/linux.js
@@ -14,6 +14,7 @@ const { BasicMessages, AnsiEscapes } = require('./log');
const homedir = execSync('grep $(logname) /etc/passwd | cut -d ":" -f6').toString().trim();
const KnownLinuxPaths = Object.freeze([
+ '/usr/lib/discord-canary',
'/usr/share/discord-canary',
'/usr/lib64/discord-canary',
'/opt/discord-canary',
@@ -51,10 +52,10 @@ exports.getAppDir = async () => {
}
}
- return join(discordPath, 'resources', 'app');
+ return join(discordPath, 'app');
}
const discordPath = discordProcess[4].split('/');
discordPath.splice(discordPath.length - 1, 1);
- return join('/', ...discordPath, 'resources', 'app');
+ return join('/', ...discordPath, 'app');
}; Additionally, you must copy (obviously this patch would break injections for all normal discord installs, but that could be solved with a simple if statement) |
Is there any plan on merging this PR someday, or are there too many issues with it at the moment ? (Or maybe there are other more important PRs) |
@AkechiShiro It might be better to patch the injector in the |
You're right, I'm guessing to do such a patch, following this course of action is right ? The changes to do on the PKGBUILD are probably add two Edit: I actually noticed I missed one more line that needs to be added inside the diff. |
Having tried the patch proposed, it doesn't look like it worked for me for some reason. Running the modified
If anyone has a clue on how to fix this. |
This PR doesn't actually fix the issue at hand, it just detects the problem and fails gracefully. I think it would be best if discussion on a real solution was in its own issue, ideally with this PR merged in the meantime. |
@AkechiShiro Make sure you run |
I had an issue injecting Powercord into the
discord-canary-electron-bin
AUR package. After some troubleshooting, I found that the problem was a discrepancy in the way that this specific package installed Discord Canary.This adds a check which informs the user that their Discord install is unusual.