-
Notifications
You must be signed in to change notification settings - Fork 9
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
Links in Expanded "Learn More" Section Not Functional #13
Comments
I used Windsurf + GPT 4o to fix this issue in a fork. I'm not a programmer so I can't review the code if it's good, but it fixed all the links to open in my system's default browser when clicked. Here's the commit that did it: ninjaeon@da2576a |
I've created a fork of the project that implements two potentially useful features:
Release: You can find pre-built binaries for Windows, Linux, and macOS on the release page of my fork: https://github.com/ninjaeon/gemini-desktop-fork Important Notes:
Contribution: If these features are deemed valuable to the main project, I would be happy to submit a pull request for consideration. Any feedback or suggestions for improvement are welcome. |
The code doesn’t work as intended, but thanks for trying! Seriously, it's an impressive effort for an AI. However, after reviewing the differences main...ninjaeon:gemini-desktop-fork:main , it’s clear that the implementation doesn’t perform a real update—it simply redirects the user to an incorrect GitHub page. Let me provide an example, focusing specifically on the auto-update feature. I’ll regard everything else as "out of scope". When I looked into the changes, I noticed several questionable decisions, like the following: Example 1// Intercept window.open calls
window.open = (url) => {
if (url && url.startsWith('http')) {
shell.openExternal(url).catch(console.error);
}
return null;
};
// Handle new window events
window.addEventListener('new-window', (e) => {
e.preventDefault();
if (e.url && e.url.startsWith('http')) {
shell.openExternal(e.url).catch(console.error);
}
});
Another issue is the extensive logging introduced in the code. The original implementation barely included logs, likely because there’s no logger, and regular users wouldn’t have access to them. These additional logs are mostly useless in the current state. Moreover, I couldn’t find evidence of a proper auto-update implementation. Based on my review, it seems to rely on events like Even if auto-update were functional, the error-handling logic for the Finally, the most frustrating aspect is the apparent attempt to claim/override authorship of the project. This is completely unacceptable and undermines your credibility. |
Thank you for the feedback! I'm not a programmer and learning as I go, so this helps immensely! The auto-update is setup to work with the fork repo, otherwise I would had no way of testing that it works (that I'm currently aware of yet). So far I've confirmed that it works on Windows since I've pushed version fork-3. I'm assuming that if there's a pull request (I've never done a pull request in my life, hence the "assumption"), the maintainer would edit the repo address from to fork's address back to the original repo's address. The logging comes from Windsurf (via Sonnet 3.5 or GPT-4o) adding it while debugging. Prompting the AI to add a feature seldom works the first time around, so when I tell it that something is broken, it adds logging so I can copy and paste error messages from the builds. The error messages are shown in command prompt when you start the app from there. Then it debugs based on the error messages. Repeat this until the new feature is fixed and added successfully. The logging is sometimes automatically removed by the AI, and in other times its not unless explicitly prompted to. I'm going prompt to try to remove all logging in the next version. To make clear to anyone that thinks I'm trying to claim "authorship" of this, I'm removing my username anywhere in the code (with exception of references to the fork repo URL so the updater continues to work with the fork until if and when the additional features get merged with the parent repo). The AI chats say I'm supposed to put my name in this as a contributor for my modifications (anytime you see my username in the code, it was suggested by AI, especially when I was setting up Github Actions workflows for Linux builds), but it doesn't matter to me. I want to learn how to do this right, and I will defer to human criticism. The original project lists ISC license in the package.json so I'm trying to adhere to that (while trying to learn how to do that properly). I personally use this app daily, so that was my primary motivation for the fork. Putting the code up is my attempt to learn how to Github and hopefully provide some value to others in the process. I appreciate the efforts of real programmers like nekupaw and yourself, as well as the rest of the open source community! And for your specific criticisms of the code, I will be copying and pasting what you wrote as a prompt and see if Windsurf can act on it (I think it should). THANK YOU! |
To avoid the perception that I was trying to claim/override authorship of the project nekupaw#13 (comment)
To make clear to anyone that thinks I'm trying to claim "ownership" of this, I've removed my username. The AI chats say I'm supposed to put my name in this as a contributor for my modifications (anytime you see my username in the code, it was suggested by AI), but it doesn't matter to me. I just want to learn how to do this right, and I will defer to human criticism. The original project lists ISC license in the package.json so I'm trying to adhere to that.
Since the electron-updater implementation worked for me, I didn't question the code. But on further review (based on my limited comprehension), it seems to be adhering to https://www.electron.build/auto-update |
To avoid the perception of "apparent attempt to claim/override authorship of the project" nekupaw#13 (comment)
Made changes to this when I removed console error logging: ninjaeon@18d4a00#diff-032e180739c4103bc3c3e71de4e372ef41482cfedaf727481a9f0724a5763f5bR20 That said, when I put your full comment on this code in the prompt, the AI agrees with you, then changes the code into something that completely breaks links from opening at all. I went around and around with it for a while and had to give up - it's beyond my (prompting) abilities for now. I'll revisit this when Windsurf upgrades their models (hopefully Gemini 2.0 Flash soon?). I had similar problems when I first tried getting the external link to open originally...it took a lot of prompting and failed builds (read: hours of trial and error) to get something that worked. I pushed a new release that removed the console error logging. Initially it broke the updater since it was relying on the console logging, but now it's updated and uses dialog boxes for errors. Since the updater code has changed, I am back to not being 100% sure the updater is working until the next release (it was before). |
Description:
Clicking on links within the expanded "Learn More" section of a response does not open the linked content.
Steps to Reproduce:
Expected Result:
The link should open in the user's default web browser.
Actual Result:
Nothing happens when the link is clicked.
The text was updated successfully, but these errors were encountered: