Skip to content

Conversation

@stm2
Copy link

@stm2 stm2 commented Aug 24, 2025

Neue Shortcodes:

  • crmap kann einen CR als SVG anzeigen, klickbar mit (teilweise) Regions- und Einheitsdetails.
  • readnr/shownr zeigen nr and, auch abschnittsweise, zeilenweise oder einzelne Regionen oder Einheiten
  • orderfile zeigt eine Befehlsdatei schön formatiert inklusive Wiki-Links und markdown in Kommentaren an

css/js only included if actually needed.

stm2 added 4 commits August 24, 2025 14:35
- crmap: show CR as interactive svg map
- display map region and unit information and commands on click
- orderfile: render orders with wiki links and comments (with markdown)
- readnr/shownr: parse NR and detect sections, regions, units
- display sections, regions units
Copilot AI review requested due to automatic review settings August 24, 2025 13:31
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request introduces new shortcodes for displaying Eressea game data in an Eleventy static site. The main purpose is to add interactive map visualization and formatted display of game reports and order files.

  • Adds three new shortcode categories: crmap for interactive SVG maps, readnr/shownr for displaying report sections, and orderfile for formatted order display
  • Implements conditional CSS/JS loading to only include assets when actually needed
  • Adds comprehensive report parsing with bookmarks and unit detection for easy navigation

Reviewed Changes

Copilot reviewed 6 out of 244 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
crs/crs.js Main plugin file implementing all shortcodes with CR file parsing, SVG generation, and report processing
crs/crs.css Styling for maps, tooltips, order files, and report displays
crs/crs-passthrough.js Client-side JavaScript for interactive map functionality and tooltips
_includes/base-layout.njk Template updated to conditionally include CRS assets and fix title display
.eleventyignore Updated to ignore template directory
.eleventy.js Plugin registration and configuration updates

Comment on lines +4 to +5
const { start } = require('repl');

Copy link

Copilot AI Aug 24, 2025

Choose a reason for hiding this comment

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

The start import from 'repl' is unused throughout the file. This import should be removed as it's not needed for the functionality.

Suggested change
const { start } = require('repl');

Copilot uses AI. Check for mistakes.
Comment on lines +930 to +931
const fs = require('fs');
const path = require('path');
Copy link

Copilot AI Aug 24, 2025

Choose a reason for hiding this comment

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

The require statements for 'fs' and 'path' should be moved to the top of the file with other imports, as they're already imported there. This avoids redundant module loading on each function call.

Suggested change
const fs = require('fs');
const path = require('path');

Copilot uses AI. Check for mistakes.
Comment on lines +9 to +12
{% if content.indexOf('crs-requires-css') !== -1 %}
<link rel="stylesheet" href="{{ '/css/crs.css' }}">
{% endif %}
{% if content.indexOf('crs-requires-js') !== -1 %}
Copy link

Copilot AI Aug 24, 2025

Choose a reason for hiding this comment

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

The conditional asset loading uses string search on content, which could have false positives if the marker class appears in text content. Consider using a more robust approach like setting template variables in the shortcodes.

Suggested change
{% if content.indexOf('crs-requires-css') !== -1 %}
<link rel="stylesheet" href="{{ '/css/crs.css' }}">
{% endif %}
{% if content.indexOf('crs-requires-js') !== -1 %}
{% if crs_requires_css %}
<link rel="stylesheet" href="{{ '/css/crs.css' }}">
{% endif %}
{% if crs_requires_js %}

Copilot uses AI. Check for mistakes.
@stm2 stm2 mentioned this pull request Aug 24, 2025
Closed
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.

1 participant