Skip to content

Conversation

@adminohg
Copy link

Note: Some shortcuts don't work after a long researh without a solution:

control + shift + n
control + alt + x

all multi key conbination are failing.

@hermeslmohg hermeslmohg force-pushed the feature/disable-chromium-shortcuts branch from a1f6c14 to 7227de8 Compare October 24, 2025 18:51
@guysoft
Copy link
Owner

guysoft commented Oct 28, 2025

Hey.
please

  1. Don't change the version of FullPageOS, that is manged by me and should not be part of the PR, Tell your LLM to look at the commit history if is not you who did this.
  2. Don't include your own intellij config setup . if you want to add .xz to gitignore, that is fine, but do so in a separate PR.
  3. I am not going to disable by default all the key shortcuts. you are welcome to add this feature with the default of "no". Other people use FullPageOS and the default was not to block all the key combination and this would break their use case.
  4. Dont use CURRENT_USER, that looks like something LLM stuck that didnt read the code. add a rule for it to use BASE_USER that is defined here: https://github.com/guysoft/CustomPiOS/blob/devel/src/modules/base/config#L35 . If you are using an LLM I suggest you also add CustomPiOS as a reference for it (some of them were trained on it, but not so well).

Thanks for your contribution, its not a long loop for this to go in NOT as default.

BTW, I am guessing you are using copilot because:

#Disable keyboard shortcuts like (CTRL+w,CTRL+N,CTRL+t) for Chromium

Here you didn't add a space between the # and the D, and all the other additions have the correct convention of having a space in between.

@hermeslmohg hermeslmohg force-pushed the feature/disable-chromium-shortcuts branch from 7227de8 to 2eac1f8 Compare October 28, 2025 15:22
Note: Some shortcuts don't work after a long researh without a solution:

control + shift + n
control + alt + x

all multi key conbination are failing.
@hermeslmohg hermeslmohg force-pushed the feature/disable-chromium-shortcuts branch from 2eac1f8 to 818b94e Compare October 28, 2025 15:50
@adminohg
Copy link
Author

Hi @guysoft,

I was working on your recommendations, and I want to apologize for my mistake; I included some changes that were not intended for this repository in this PR.

At our organization, we’re actively developing several internal features for this project. As part of that process, we use an LLM to help generate and extend certain feature add-ons. However, the main development is still done through human engineering and manual code review to ensure proper understanding of the source code.

  1. Versioning management should remain under your control and not be part of this PR.
  2. The feature should remain disabled by default and optional, since it’s only required for internal use in our organization.

I really appreciate your understanding and the opportunity to collaborate. This is a great project, and I plan to continue contributing periodically and in alignment with its core design principles.

BTW, we’re using several LLMs like Claude, ChatGPT, Gemini, but not Copilot.

Best regards,
Hermes Lorenzo

@guysoft
Copy link
Owner

guysoft commented Oct 29, 2025

Ok, Hermes, cool.

I am not sure if I missed this on the first run (I don't think so, but maybe?) but now you now have:

  1. lots of logic, including running apt list in src/modules/fullpageos/filesystem/opt/custompios/scripts/start_chromium_browser.
    That's a script that runs on each boot. And its time consuming and generally really not a good idea to be looking in a database if the packages is installed. You can just see if the configuration and binary exists in the filesystem.
  2. You are generating a temp file for no reason in /tmp with the bindings, again every boot.
  3. you have a bug - you didn't test this. Because $BASE_USER does not exist in the scope of the rpi filesystem, only the build process, again its an ai that can tell that this builds and then runs somewhere else, you can tell it to build and then look in the OS. However this is not enough - please test this before you change because if I merge a broken bit of code it will break for everyone including you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants