Skip to content

fix: remove unnecessary grep -E flag and clarify Adminer warning message - #218

Merged
PDowney merged 3 commits into
masterfrom
copilot/fix-grep-command-usage
Apr 30, 2026
Merged

fix: remove unnecessary grep -E flag and clarify Adminer warning message#218
PDowney merged 3 commits into
masterfrom
copilot/fix-grep-command-usage

Conversation

Copilot AI commented Apr 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Two minor cleanup fixes in admin-control-panel-install.sh for the Adminer card removal logic.

Software Version Updates

Changed Versions

No version changes — code quality fixes only.

Version Diff

-    if grep -qE '<div[^>]*id="adminer-tool"[^>]*>' "/var/www/admin/control-panel/index.html"; then
+    if grep -q '<div[^>]*id="adminer-tool"[^>]*>' "/var/www/admin/control-panel/index.html"; then

-        echo "Warning: Expected Adminer tool div not found in index.html; skipping Adminer card removal." >&2
+        echo "Warning: Expected <div> with id=\"adminer-tool\" not found in index.html; skipping Adminer card removal." >&2

Verification Checklist

  • -E flag removed — pattern uses only BRE-compatible syntax ([^>]*), ERE not needed
  • Warning message now references the exact HTML attribute (id="adminer-tool") being searched, making it immediately actionable for diagnosis
  • CHANGELOG.md updated

Notes

The grep -E flag was inconsistent with the rest of the script and unnecessary for this pattern. The updated warning message reduces ambiguity when the expected HTML structure is not found during Adminer removal.

Original prompt 
Please apply the following diffs and create a pull request.
Once the PR is ready, give it a title based on the messages of the fixes being applied.

[{"message":"The grep command uses extended regex (`-E`) but the pattern doesn't require it. The pattern `<div[^>]*id=\\\"adminer-tool\\\"[^>]*>` can be matched with basic regex. Consider removing the `-E` flag for consistency with the rest of the script, or if extended regex is intentional, document why it's needed for this specific pattern.","fixFiles":[{"filePath":"scripts/install/tools/frontend/admin-control-panel-install.sh","diff":"diff --git a/scripts/install/tools/frontend/admin-control-panel-install.sh b/scripts/install/tools/frontend/admin-control-panel-install.sh\n--- a/scripts/install/tools/frontend/admin-control-panel-install.sh\n+++ b/scripts/install/tools/frontend/admin-control-panel-install.sh\n@@ -52,7 +52,7 @@\n     # If this structure changes, update this command (or switch to an HTML-aware tool) to avoid partial removal.\n     # To avoid corrupting the page if the structure has changed, first ensure that the expected\n     # single-line opening <div> for the Adminer card is present before applying the sed range.\n-    if grep -qE '<div[^>]*id=\"adminer-tool\"[^>]*>' \"/var/www/admin/control-panel/index.html\"; then\n+    if grep -q '<div[^>]*id=\"adminer-tool\"[^>]*>' \"/var/www/admin/control-panel/index.html\"; then\n         # Extract the block that would be deleted, then perform a simple sanity check\n         # to ensure there are no nested <div> elements that would cause a partial removal.\n         adminer_block=\"$(\n"}]},{"message":"The warning message refers to 'Adminer tool div' but the script logic is checking for the `id=\\\"adminer-tool\\\"` attribute. For clarity and future maintenance, consider making the error message more specific about what HTML structure is expected, such as 'Warning: Expected div with id=\\\"adminer-tool\\\" not found in index.html'.","fixFiles":[{"filePath":"scripts/install/tools/frontend/admin-control-panel-install.sh","diff":"diff --git a/scripts/install/tools/frontend/admin-control-panel-install.sh b/scripts/install/tools/frontend/admin-control-panel-install.sh\n--- a/scripts/install/tools/frontend/admin-control-panel-install.sh\n+++ b/scripts/install/tools/frontend/admin-control-panel-install.sh\n@@ -66,7 +66,7 @@\n             echo \"Warning: Adminer tool block appears to contain nested <div> elements; skipping Adminer card removal to avoid corrupting index.html.\" >&2\n         fi\n     else\n-        echo \"Warning: Expected Adminer tool div not found in index.html; skipping Adminer card removal.\" >&2\n+        echo \"Warning: Expected <div> with id=\\\"adminer-tool\\\" not found in index.html; skipping Adminer card removal.\" >&2\n \n     fi\n fi\n"}]}]

Copilot AI changed the title [WIP] Fix grep command to use basic regex in admin control panel install script fix: remove unnecessary grep -E flag and clarify Adminer warning message Apr 30, 2026
Copilot AI requested a review from PDowney April 30, 2026 02:24
@PDowney
Remove outdated changelog entry for admin control panel
ba386e0 
Removed outdated entry for admin control panel install cleanup.
@github-actions

github-actions Bot commented Apr 30, 2026

Copy link
Copy Markdown

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

@PDowney
PDowney marked this pull request as ready for review April 30, 2026 02:27
Copilot AI review requested due to automatic review settings April 30, 2026 02:27
@sonarqubecloud

sonarqubecloud Bot commented Apr 30, 2026

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Copilot AI reviewed Apr 30, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR makes small maintainability/clarity tweaks to the Adminer card removal guard logic in the admin control panel installer script.

Changes:

  • Remove unnecessary grep -E usage for a pattern that works with basic regular expressions.
  • Clarify the warning message to reference the exact <div id="adminer-tool"> structure being searched.

Comment thread scripts/install/tools/frontend/admin-control-panel-install.sh
# To avoid corrupting the page if the structure has changed, first ensure that the expected
# single-line opening <div> for the Adminer card is present before applying the sed range.
if grep -qE '<div[^>]*id="adminer-tool"[^>]*>' "/var/www/admin/control-panel/index.html"; then
if grep -q '<div[^>]*id="adminer-tool"[^>]*>' "/var/www/admin/control-panel/index.html"; then

Copilot AI Apr 30, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR description/checklist indicates CHANGELOG.md was updated, but this PR diff only changes admin-control-panel-install.sh. Either include the corresponding CHANGELOG.md update (per project standard) or update the PR description/checklist to match what actually changed.

Copilot uses AI. Check for mistakes.
@PDowney
PDowney merged commit 391d364 into master Apr 30, 2026
14 checks passed
@github-actions
github-actions Bot deleted the copilot/fix-grep-command-usage branch April 30, 2026 03:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
+1 more reviewer
@PDowney PDowney PDowney approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@PDowney PDowney

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@PDowney