KDE Plasma package has been added to the script. #219
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Adding plasma packages
Adding plasma packages to vital
update the KDE plasma 6 build step
Reviewer's GuideThis PR adds support for building GhostBSD with KDE Plasma 6 by extending the documentation with a new build option, introducing a dedicated plasma.sh configuration script for SDDM and xinit, and creating placeholder directories for Plasma packages. Sequence Diagram for Plasma Desktop Configuration via plasma.shsequenceDiagram
actor User
participant B as "build.sh"
participant P as "desktop_config/plasma.sh"
participant SF as "System Files & Configs"
User->>B: Execute ./build.sh -d plasma
B->>P: Invoke for Plasma setup
P->>P: Source common_config/*sh
P->>SF: Apply generic patches (patch_etc_files)
P->>SF: Setup live user (community_setup_liveuser)
P->>SF: Setup autologin (community_setup_autologin)
P->>P: Perform SDDM setup (sddm_setup)
activate P
P->>SF: Configure /etc/sddm.conf (Autologin, Theme, Session)
deactivate P
P->>P: Perform Xinit setup (setup_xinit)
activate P
P->>SF: Configure kscreenlockerrc for live_user (disable autolock)
P->>SF: Configure .xinitrc for live_user, root, skel (startplasma-x11)
deactivate P
P->>SF: Apply final setup (final_setup)
P-->>B: Plasma configuration complete
B-->>User: GhostBSD image with KDE Plasma built
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @SunilSherkhane - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `desktop_config/plasma.sh:6` </location>
<code_context>
+set -e -u
+
+# Source common configuration scripts
+. "${cwd}/common_config/autologin.sh"
+. "${cwd}/common_config/base-setting.sh"
+. "${cwd}/common_config/finalize.sh"
</code_context>
<issue_to_address>
Undefined variable 'cwd' used for sourcing
Please define 'cwd' before using it, such as with cwd=$(dirname "$0"), or use an explicit path.
</issue_to_address>
### Comment 2
<location> `desktop_config/plasma.sh:48` </location>
<code_context>
+ mkdir -p /home/${live_user}/.config
+ kwriteconfig5 --file /home/${live_user}/.config/kscreenlockerrc --group Daemon --key Autolock false
+ kwriteconfig5 --file /home/${live_user}/.config/kscreenlockerrc --group Daemon --key LockOnResume false
+ echo 'exec ck-launch-session startplasma-x11' >> /home/${live_user}/.xinitrc
+ "
+
</code_context>
<issue_to_address>
Appending .xinitrc exec line without idempotency check
This can cause duplicate lines; check if the line exists before appending.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
kwriteconfig5 --file /home/${live_user}/.config/kscreenlockerrc --group Daemon --key LockOnResume false
echo 'exec ck-launch-session startplasma-x11' >> /home/${live_user}/.xinitrc
"
=======
kwriteconfig5 --file /home/${live_user}/.config/kscreenlockerrc --group Daemon --key LockOnResume false
grep -qxF 'exec ck-launch-session startplasma-x11' /home/${live_user}/.xinitrc || echo 'exec ck-launch-session startplasma-x11' >> /home/${live_user}/.xinitrc
"
>>>>>>> REPLACE
</suggested_fix>
### Comment 3
<location> `desktop_config/plasma.sh:53` </location>
<code_context>
+
+ # Set the same .xinitrc for root and skel
+ echo "exec ck-launch-session startplasma-x11" > "${release}/root/.xinitrc"
+ echo "exec ck-launch-session startplasma-x11" > "${release}/usr/share/skel/dot.xinitrc"
+}
+
</code_context>
<issue_to_address>
Skeleton file named 'dot.xinitrc' instead of '.xinitrc'
The file in the skel directory should be named '.xinitrc' (with a leading dot) to ensure it is correctly copied to new user home directories. 'dot.xinitrc' will not be recognized by default.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| set -e -u | ||
|
|
||
| # Source common configuration scripts | ||
| . "${cwd}/common_config/autologin.sh" |
There was a problem hiding this comment.
issue (bug_risk): Undefined variable 'cwd' used for sourcing
Please define 'cwd' before using it, such as with cwd=$(dirname "$0"), or use an explicit path.
Comment on lines
+47
to
+49
| kwriteconfig5 --file /home/${live_user}/.config/kscreenlockerrc --group Daemon --key LockOnResume false | ||
| echo 'exec ck-launch-session startplasma-x11' >> /home/${live_user}/.xinitrc | ||
| " |
There was a problem hiding this comment.
suggestion (bug_risk): Appending .xinitrc exec line without idempotency check
This can cause duplicate lines; check if the line exists before appending.
Suggested change
| kwriteconfig5 --file /home/${live_user}/.config/kscreenlockerrc --group Daemon --key LockOnResume false | |
| echo 'exec ck-launch-session startplasma-x11' >> /home/${live_user}/.xinitrc | |
| " | |
| kwriteconfig5 --file /home/${live_user}/.config/kscreenlockerrc --group Daemon --key LockOnResume false | |
| grep -qxF 'exec ck-launch-session startplasma-x11' /home/${live_user}/.xinitrc || echo 'exec ck-launch-session startplasma-x11' >> /home/${live_user}/.xinitrc | |
| " |
|
|
||
| # Set the same .xinitrc for root and skel | ||
| echo "exec ck-launch-session startplasma-x11" > "${release}/root/.xinitrc" | ||
| echo "exec ck-launch-session startplasma-x11" > "${release}/usr/share/skel/dot.xinitrc" |
There was a problem hiding this comment.
issue (bug_risk): Skeleton file named 'dot.xinitrc' instead of '.xinitrc'
The file in the skel directory should be named '.xinitrc' (with a leading dot) to ensure it is correctly copied to new user home directories. 'dot.xinitrc' will not be recognized by default.
|
Sunil, are you volunteering to take responsibility for addressing all future KDE Plasma-related questions and issues? If we are to include KDE Plasma as an option, we need a dedicated technical lead to manage and support it. We want to avoid placing the burden on Eric to troubleshoot and maintain every desktop environment offered. |
|
@SunilSherkhane I am waiting only to answer @xcrsz question. |
Sure, I will answer the questions |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
KDE Plasma package has been added to the script. Tried and tested and it works normally. need to modify common packages and move it to DE Specific
Summary by Sourcery
Add KDE Plasma 6 as a supported desktop environment in the build script with its own configuration and packaging directories.
New Features:
Documentation: