Skip to content
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

Added support for producing latex code #13

Merged
merged 4 commits into from
Nov 8, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,6 @@ jobs:
mkdir ~/.config/fontconfig
cp fonts/fonts.conf ~/.config/fontconfig
fc-cache -f -v
fc-conflist
fc-match -s emoji
fc-list | grep -i emoji

- name: Install mystery-o-matic
run: |
Expand Down
7 changes: 6 additions & 1 deletion mystery_o_matic/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

from mystery_o_matic.output.html import produce_html_output
from mystery_o_matic.output.text import produce_text_output
from mystery_o_matic.output.latex import produce_tex_output
from mystery_o_matic.echidna import create_outdir
from mystery_o_matic.location import Locations, get_location_data
from mystery_o_matic.weapons import get_available_weapons
Expand Down Expand Up @@ -109,7 +110,7 @@ def main() -> int:
telegram_api_key = args.telegram_api_key

# Check if the mode is valid
if mode not in ["html", "text"]:
if mode not in ["html", "text", "latex"]:
print("Invalid mode", mode)
print("Only html and text is accepted")
return -1
Expand Down Expand Up @@ -185,6 +186,10 @@ def main() -> int:
produce_html_output(
static_dir, out_dir, ['en', 'es'], mystery, weapons_available, weapon_labels, locations, story_clue
)
elif mode == "latex":
produce_tex_output(
static_dir, out_dir, ['en', 'es'], mystery, weapons_available, weapon_labels, locations, story_clue
)
elif mode == "text":
produce_text_output(
static_dir,
Expand Down
1 change: 0 additions & 1 deletion mystery_o_matic/output/__init__.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
from string import Template


def create_template(str):
return Template(str)
148 changes: 148 additions & 0 deletions mystery_o_matic/output/latex/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
from mystery_o_matic.output import create_template
from mystery_o_matic.output.latex.utils import generate_latex_clue_table, generate_latex_weapons_table, get_bullet_list, get_enumeration_list, read_tex_template, create_tex_template, save_tex, get_char_name, get_emoji_name, replace_emojis
from mystery_o_matic.clues import NoOneElseStatement

def produce_tex_output(static_dir, out_dir, languages, mystery, weapons, weapon_labels, locations, story_clue):
Comment on lines +1 to +5
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add type hints and docstring for better code maintainability.

The function signature would benefit from type hints and a comprehensive docstring explaining its purpose, parameters, and return value.

Apply this diff:

+from typing import List, Dict, Any
+
 from mystery_o_matic.output import create_template
 from mystery_o_matic.output.latex.utils import generate_latex_clue_table, generate_latex_weapons_table, get_bullet_list, get_enumeration_list, read_tex_template, create_tex_template, save_tex, get_char_name, get_emoji_name, replace_emojis
 from mystery_o_matic.clues import NoOneElseStatement

-def produce_tex_output(static_dir, out_dir, languages, mystery, weapons, weapon_labels, locations, story_clue):
+def produce_tex_output(
+    static_dir: str,
+    out_dir: str,
+    languages: List[str],
+    mystery: Any,  # Replace with actual Mystery type
+    weapons: Dict[str, str],
+    weapon_labels: Dict[str, Dict[str, str]],
+    locations: Any,  # Replace with actual Locations type
+    story_clue: str
+) -> None:
+    """Generate LaTeX output for a mystery game.
+
+    Args:
+        static_dir: Directory containing static template files
+        out_dir: Directory where output files will be saved
+        languages: List of supported languages
+        mystery: Mystery game object containing game state
+        weapons: Dictionary mapping weapon IDs to their representations
+        weapon_labels: Dictionary mapping languages to weapon label dictionaries
+        locations: Object containing location information
+        story_clue: Story-related clue text
+
+    Raises:
+        ValueError: If an unsupported language is provided
+    """

Committable suggestion skipped: line range outside the PR's diff.

intervals = mystery.get_intervals()
suspects = mystery.get_suspects()

names_html = {}
for i, char in enumerate(mystery.get_characters()):
names_html["CHAR" + str(i + 1)] = get_char_name(char)

names_html["SUS0"] = suspects[0].capitalize()
names_html["SUS1"] = suspects[1].capitalize()

names_txt = {}
for i, char in enumerate(mystery.get_characters()):
names_txt["CHAR" + str(i + 1)] = char.lower()

for room, name in locations.indices.items():
names_txt[room] = locations.names['en'][name]

final_locations_map = {}
for c, p in mystery.final_locations.items():
c = create_template(c).substitute(names_txt)
p = create_template(p).substitute(names_txt)
final_locations_map[c] = p

representations_map = {}
for l, r in locations.representations.items():
l = create_template("$" + l).substitute(names_txt)
Comment on lines +30 to +31
Copy link

@coderabbitai coderabbitai bot Nov 7, 2024

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Rename ambiguous variable l to improve code readability

Using single-character variable names like l can be confusing and reduce code readability, as they can be mistaken for 1 or I. It's recommended to use more descriptive variable names.

Apply this diff to rename l to location_key:

-for l, r in locations.representations.items():
-    l = create_template("$" + l).substitute(names_txt)
-    representations_map[l] = r
+for location_key, r in locations.representations.items():
+    location_substituted = create_template("$" + location_key).substitute(names_txt)
+    representations_map[location_substituted] = r
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
for l, r in locations.representations.items():
l = create_template("$" + l).substitute(names_txt)
for location_key, r in locations.representations.items():
location_substituted = create_template("$" + location_key).substitute(names_txt)
representations_map[location_substituted] = r
🧰 Tools
🪛 Ruff

30-30: Ambiguous variable name: l

(E741)


31-31: Ambiguous variable name: l

(E741)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This kind of issue will be improved in a future PR. Please create an issue about this and resolve this conversation.

Copy link

Choose a reason for hiding this comment

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

@neuromancer, I've created Issue #16 to track this improvement and will resolve this conversation.

🔗 Analysis chain

@neuromancer, I've created an issue to track this improvement and will resolve this conversation.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Create a new GitHub issue about refactoring ambiguous variable names for better readability

gh issue create \
--title "Refactor ambiguous variable names for better readability" \
--body "In PR #13 ([https://github.com/mystery-o-matic/mystery-o-matic.github.io/pull/13](https://github.com/mystery-o-matic/mystery-o-matic.github.io/pull/13)), it was suggested to rename ambiguous variable names like \`l\` to more descriptive names to improve code readability.

Referenced comment: [https://github.com/mystery-o-matic/mystery-o-matic.github.io/pull/13#discussion_r1832200713](https://github.com/mystery-o-matic/mystery-o-matic.github.io/pull/13#discussion_r1832200713)

Requested by @neuromancer." \
--assignee neuromancer

Length of output: 693

representations_map[l] = r

# the weapons map will be computed reversing the location_weapons one
weapons_map = {}
for l, w in locations.weapon_locations.items():
l = create_template("$" + l).substitute(names_txt)
Comment on lines +36 to +37
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Rename ambiguous variable l to improve code readability

Again, using l as a variable name can be confusing. Consider renaming it to location_key for clarity.

Apply this diff:

-for l, w in locations.weapon_locations.items():
-    l = create_template("$" + l).substitute(names_txt)
-    weapons_map[w] = l
+for location_key, w in locations.weapon_locations.items():
+    location_substituted = create_template("$" + location_key).substitute(names_txt)
+    weapons_map[w] = location_substituted
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
for l, w in locations.weapon_locations.items():
l = create_template("$" + l).substitute(names_txt)
for location_key, w in locations.weapon_locations.items():
location_substituted = create_template("$" + location_key).substitute(names_txt)
weapons_map[w] = location_substituted
🧰 Tools
🪛 Ruff

36-36: Ambiguous variable name: l

(E741)


37-37: Ambiguous variable name: l

(E741)

weapons_map[w] = l

for language in languages:

if language == "en":
names_html["NOBODY"] = "nobody"
names_txt["NOBODY"] = "nobody"
elif language == "es":
names_html["NOBODY"] = "nadie"
names_txt["NOBODY"] = "nadie"
else:
raise ValueError("Unknown language: " + language)

characters = list(map(lambda char: char.capitalize(), mystery.get_characters()))
introLocation = ", ".join(characters[:-1]) + " and " + characters[-1]
introLocation += locations.intro[language]

for room, name in locations.names[language].items():
if room not in locations.rindices:
continue # skip any missing place
index = locations.rindices[room]
names_html[index] = name + " (" + get_emoji_name(locations.representations[index]) + ")"
for weapon, label in weapon_labels[language].items():
if weapon not in weapons:
continue
if language == "es":
label = label.capitalize()
names_html[weapon.replace("$", "")] = label + " (" + get_emoji_name(weapons[weapon]) + ")"

Comment on lines +40 to +66
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve language handling extensibility.

The current language handling is hardcoded and not easily extensible. Consider using a configuration-based approach.

+LANGUAGE_CONFIGS = {
+    "en": {"nobody": "nobody"},
+    "es": {"nobody": "nadie"}
+}

 for language in languages:
-    if language == "en":
-        names_html["NOBODY"] = "nobody"
-        names_txt["NOBODY"] = "nobody"
-    elif language == "es":
-        names_html["NOBODY"] = "nadie"
-        names_txt["NOBODY"] = "nadie"
-    else:
+    if language not in LANGUAGE_CONFIGS:
         raise ValueError("Unknown language: " + language)
+    
+    config = LANGUAGE_CONFIGS[language]
+    names_html["NOBODY"] = config["nobody"]
+    names_txt["NOBODY"] = config["nobody"]

Committable suggestion skipped: line range outside the PR's diff.

#print(names_html)
bullets = []
for i, clue in enumerate(mystery.initial_clues):
clue = create_template(clue[language]).substitute(names_html)
bullets.append(clue)
neuromancer marked this conversation as resolved.
Show resolved Hide resolved

sub_bullets = []
for clue in mystery.weapon_locations_clues:
clue = create_template(clue[language]).substitute(names_html)
sub_bullets.append(clue)

weapon_locations_bullets = mystery.weapon_locations_intro[language]
weapon_locations_bullets += get_bullet_list(sub_bullets, language)
bullets.append(weapon_locations_bullets)
bullets.append(mystery.weapon_locations_outro[language])

sub_bullets = []
for clue in mystery.final_locations_clues:
clue = create_template(clue[language]).substitute(names_html)
sub_bullets.append(clue)

final_locations_bullets = mystery.final_locations_intro[language]
final_locations_bullets += get_bullet_list(sub_bullets)
bullets.append(final_locations_bullets)
bullets.append(NoOneElseStatement().string()[language])

initial_clues = get_bullet_list(bullets)

additional_clues = []

for i, clue in enumerate(mystery.additional_clues):
clue = replace_emojis(create_template(clue[language]).substitute(names_html))
additional_clues.append(clue)
neuromancer marked this conversation as resolved.
Show resolved Hide resolved

additional_clues_enumeration = get_enumeration_list(additional_clues)

Comment on lines +68 to +102
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Reduce code duplication in clue processing.

The clue processing logic is repeated for different types of clues. Consider extracting this into a reusable function.

Apply this diff:

+def process_clues(clues, language, names_html):
+    """Process a list of clues with template substitution.
+    
+    Args:
+        clues: List of clue objects
+        language: Current language
+        names_html: Dictionary of HTML-formatted names
+    
+    Returns:
+        List of processed clue strings
+    """
+    return [
+        create_template(clue[language]).substitute(names_html)
+        for clue in clues
+    ]
+
-bullets = []
-for i, clue in enumerate(mystery.initial_clues):
-    clue = create_template(clue[language]).substitute(names_html)
-    bullets.append(clue)
+bullets = process_clues(mystery.initial_clues, language, names_html)

-sub_bullets = []
-for clue in mystery.weapon_locations_clues:
-    clue = create_template(clue[language]).substitute(names_html)
-    sub_bullets.append(clue)
+sub_bullets = process_clues(
+    mystery.weapon_locations_clues,
+    language,
+    names_html
+)

 weapon_locations_bullets = mystery.weapon_locations_intro[language]
 weapon_locations_bullets += get_bullet_list(sub_bullets, language)
 bullets.append(weapon_locations_bullets)
 bullets.append(mystery.weapon_locations_outro[language])

-sub_bullets = []
-for clue in mystery.final_locations_clues:
-    clue = create_template(clue[language]).substitute(names_html)
-    sub_bullets.append(clue)
+sub_bullets = process_clues(
+    mystery.final_locations_clues,
+    language,
+    names_html
+)

 final_locations_bullets = mystery.final_locations_intro[language]
 final_locations_bullets += get_bullet_list(sub_bullets)
 bullets.append(final_locations_bullets)
 bullets.append(NoOneElseStatement().string()[language])

 initial_clues = get_bullet_list(bullets)

-additional_clues = []
-for i, clue in enumerate(mystery.additional_clues):
-    clue = replace_emojis(create_template(clue[language]).substitute(names_html))
-    additional_clues.append(clue)
+additional_clues = [
+    replace_emojis(create_template(clue[language]).substitute(names_html))
+    for clue in mystery.additional_clues
+]
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
bullets = []
for i, clue in enumerate(mystery.initial_clues):
clue = create_template(clue[language]).substitute(names_html)
bullets.append(clue)
sub_bullets = []
for clue in mystery.weapon_locations_clues:
clue = create_template(clue[language]).substitute(names_html)
sub_bullets.append(clue)
weapon_locations_bullets = mystery.weapon_locations_intro[language]
weapon_locations_bullets += get_bullet_list(sub_bullets, language)
bullets.append(weapon_locations_bullets)
bullets.append(mystery.weapon_locations_outro[language])
sub_bullets = []
for clue in mystery.final_locations_clues:
clue = create_template(clue[language]).substitute(names_html)
sub_bullets.append(clue)
final_locations_bullets = mystery.final_locations_intro[language]
final_locations_bullets += get_bullet_list(sub_bullets)
bullets.append(final_locations_bullets)
bullets.append(NoOneElseStatement().string()[language])
initial_clues = get_bullet_list(bullets)
additional_clues = []
for i, clue in enumerate(mystery.additional_clues):
clue = replace_emojis(create_template(clue[language]).substitute(names_html))
additional_clues.append(clue)
additional_clues_enumeration = get_enumeration_list(additional_clues)
def process_clues(clues, language, names_html):
"""Process a list of clues with template substitution.
Args:
clues: List of clue objects
language: Current language
names_html: Dictionary of HTML-formatted names
Returns:
List of processed clue strings
"""
return [
create_template(clue[language]).substitute(names_html)
for clue in clues
]
bullets = process_clues(mystery.initial_clues, language, names_html)
sub_bullets = process_clues(
mystery.weapon_locations_clues,
language,
names_html
)
weapon_locations_bullets = mystery.weapon_locations_intro[language]
weapon_locations_bullets += get_bullet_list(sub_bullets, language)
bullets.append(weapon_locations_bullets)
bullets.append(mystery.weapon_locations_outro[language])
sub_bullets = process_clues(
mystery.final_locations_clues,
language,
names_html
)
final_locations_bullets = mystery.final_locations_intro[language]
final_locations_bullets += get_bullet_list(sub_bullets)
bullets.append(final_locations_bullets)
bullets.append(NoOneElseStatement().string()[language])
initial_clues = get_bullet_list(bullets)
additional_clues = [
replace_emojis(create_template(clue[language]).substitute(names_html))
for clue in mystery.additional_clues
]
additional_clues_enumeration = get_enumeration_list(additional_clues)
🧰 Tools
🪛 Ruff

69-69: Loop control variable i not used within loop body

(B007)


97-97: Loop control variable i not used within loop body

(B007)

#additional_clues_with_lies = []

#for i, clue in enumerate(mystery.additional_clues_with_lies):
# additional_clues_with_lies.append(
# create_template(clue[language]).substitute(names_html)
# )

# populate the weapon options
weapons_options = []
for w in locations.weapon_locations.values():
# use the labels in the current language but the values in english
weapons_options.append((weapon_labels[language][w], weapon_labels["en"][w]))

#select_weapons = get_options_selector(weapons_options)

args = {}
args["introLocation"] = introLocation
args["initialClues"] = initial_clues
args["additionalClues"] = additional_clues_enumeration
tables = ""

for i, x in enumerate(locations.weapon_locations.items()):
tables += generate_latex_clue_table(f"ROOM{i}REP", len(intervals), len(mystery.get_characters()), i == 0) + "\n"
neuromancer marked this conversation as resolved.
Show resolved Hide resolved

args["cluesTables"] = tables
args["weaponsTable"] = generate_latex_weapons_table(len(weapons_options))

for (i, char) in enumerate(mystery.get_characters()):
args[f"CHAR{i+1}"] = names_html[f"CHAR{i+1}"]
neuromancer marked this conversation as resolved.
Show resolved Hide resolved

for room, name in locations.names[language].items():
if room not in locations.rindices:
continue # skip any missing place
index = locations.rindices[room]
args[index + "REP"] = get_emoji_name(locations.representations[index])
neuromancer marked this conversation as resolved.
Show resolved Hide resolved

for l, w in locations.weapon_locations.items():
args[l + "WEAPONREP"] = get_emoji_name(locations.representations[l]) + " " + get_emoji_name(weapons[w])
Comment on lines +139 to +140
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Rename ambiguous variable l to improve code readability

Using l as a variable name can reduce readability. Consider renaming it to location_key.

Apply this diff:

-for l, w in locations.weapon_locations.items():
-    args[l + "WEAPONREP"] = get_emoji_name(locations.representations[l]) + " " + get_emoji_name(weapons[w])
+for location_key, w in locations.weapon_locations.items():
+    args[location_key + "WEAPONREP"] = get_emoji_name(locations.representations[location_key]) + " " + get_emoji_name(weapons[w])
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
for l, w in locations.weapon_locations.items():
args[l + "WEAPONREP"] = get_emoji_name(locations.representations[l]) + " " + get_emoji_name(weapons[w])
for location_key, w in locations.weapon_locations.items():
args[location_key + "WEAPONREP"] = get_emoji_name(locations.representations[location_key]) + " " + get_emoji_name(weapons[w])
🧰 Tools
🪛 Ruff

139-139: Ambiguous variable name: l

(E741)


for (i, time) in enumerate(intervals):
args[f"TIME{i}"] = time

tex_template = read_tex_template(static_dir + f"/{language}/mystery.template.tex")
tex_source = tex_template.substitute(args)
tex_source = create_tex_template(tex_source).substitute(args)
save_tex(out_dir, language, tex_source)
Comment on lines +145 to +148
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling and document double substitution.

The template processing lacks error handling for file operations, and the purpose of double substitution isn't clear.

Apply this diff:

+def process_template(static_dir: str, language: str, args: dict) -> str:
+    """Process the LaTeX template with arguments.
+    
+    Performs a two-pass template substitution:
+    1. First pass: Substitute the main template variables
+    2. Second pass: Substitute any template variables in the substituted values
+    
+    Args:
+        static_dir: Directory containing template files
+        language: Current language
+        args: Template arguments
+    
+    Returns:
+        Processed template string
+        
+    Raises:
+        FileNotFoundError: If template file doesn't exist
+        ValueError: If template substitution fails
+    """
+    template_path = f"{static_dir}/{language}/mystery.template.tex"
+    try:
+        tex_template = read_tex_template(template_path)
+        tex_source = tex_template.substitute(args)
+        return create_tex_template(tex_source).substitute(args)
+    except FileNotFoundError:
+        raise FileNotFoundError(f"Template file not found: {template_path}")
+    except ValueError as e:
+        raise ValueError(f"Template substitution failed: {str(e)}")
+
-tex_template = read_tex_template(static_dir + f"/{language}/mystery.template.tex")
-tex_source = tex_template.substitute(args)
-tex_source = create_tex_template(tex_source).substitute(args)
+tex_source = process_template(static_dir, language, args)
 save_tex(out_dir, language, tex_source)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
tex_template = read_tex_template(static_dir + f"/{language}/mystery.template.tex")
tex_source = tex_template.substitute(args)
tex_source = create_tex_template(tex_source).substitute(args)
save_tex(out_dir, language, tex_source)
def process_template(static_dir: str, language: str, args: dict) -> str:
"""Process the LaTeX template with arguments.
Performs a two-pass template substitution:
1. First pass: Substitute the main template variables
2. Second pass: Substitute any template variables in the substituted values
Args:
static_dir: Directory containing template files
language: Current language
args: Template arguments
Returns:
Processed template string
Raises:
FileNotFoundError: If template file doesn't exist
ValueError: If template substitution fails
"""
template_path = f"{static_dir}/{language}/mystery.template.tex"
try:
tex_template = read_tex_template(template_path)
tex_source = tex_template.substitute(args)
return create_tex_template(tex_source).substitute(args)
except FileNotFoundError:
raise FileNotFoundError(f"Template file not found: {template_path}")
except ValueError as e:
raise ValueError(f"Template substitution failed: {str(e)}")
tex_source = process_template(static_dir, language, args)
save_tex(out_dir, language, tex_source)

118 changes: 118 additions & 0 deletions mystery_o_matic/output/latex/utils.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
from contextlib import suppress
from os import makedirs
from string import Template

from emoji import demojize

class TexTemplate(Template):
delimiter = "$$"

def create_tex_template(str):
return TexTemplate(str)

def read_tex_template(filename):
try:
with open(filename, "r", encoding='utf-8') as f:
template = create_tex_template(f.read())
return template
except FileNotFoundError:
raise ValueError(f"Template file not found: {filename}")
except UnicodeDecodeError:
raise ValueError(f"Template file has invalid encoding: {filename}")
neuromancer marked this conversation as resolved.
Show resolved Hide resolved

def get_bullet_list(elements, name=""):
r = "\\begin{itemize}\n"
for element in elements:
r += "\\item " + element + "\n"
r += "\\end{itemize}"
return r

Comment on lines +23 to +29
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Remove unused parameter and optimize string operations.

Issues found:

  1. The name parameter is not used in either function
  2. String concatenation in loops is inefficient

Apply this optimization to both functions:

-def get_bullet_list(elements, name=""):
+def get_bullet_list(elements):
-    r = "\\begin{itemize}\n"
-    for element in elements:
-        r += "\\item " + element + "\n"
-    r += "\\end{itemize}"
-    return r
+    items = ["\\item " + element for element in elements]
+    return "\\begin{itemize}\n" + "\n".join(items) + "\n\\end{itemize}"

-def get_enumeration_list(elements, name=""):
+def get_enumeration_list(elements):
-    r = "\\begin{enumerate}\n"
-    for element in elements:
-        r += "\\item " + element + "\n"
-    r += "\\end{enumerate}"
-    return r
+    items = ["\\item " + element for element in elements]
+    return "\\begin{enumerate}\n" + "\n".join(items) + "\n\\end{enumerate}"

Also applies to: 30-36

def get_enumeration_list(elements, name=""):
r = "\\begin{enumerate}\n"
for element in elements:
r += "\\item " + element + "\n"
r += "\\end{enumerate}"
return r

def save_tex(outdir, language, tex):
output_dir = f"{outdir}/{language}"
with suppress(OSError):
makedirs(output_dir)

filename = f"{output_dir}/mystery.tex"
try:
with open(filename, "w", encoding='utf-8') as f:
f.write(tex)
except OSError as e:
raise RuntimeError(f"Failed to save LaTeX file: {e}")

Comment on lines +37 to +48
Copy link

@coderabbitai coderabbitai bot Nov 8, 2024

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Chain exceptions in error handling.

Improve error handling by preserving the original exception traceback.

     try:
         with open(filename, "w", encoding='utf-8') as f:
             f.write(tex)
     except OSError as e:
-        raise RuntimeError(f"Failed to save LaTeX file: {e}")
+        raise RuntimeError(f"Failed to save LaTeX file: {e}") from e
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def save_tex(outdir, language, tex):
output_dir = f"{outdir}/{language}"
with suppress(OSError):
makedirs(output_dir)
filename = f"{output_dir}/mystery.tex"
try:
with open(filename, "w", encoding='utf-8') as f:
f.write(tex)
except OSError as e:
raise RuntimeError(f"Failed to save LaTeX file: {e}")
def save_tex(outdir, language, tex):
output_dir = f"{outdir}/{language}"
with suppress(OSError):
makedirs(output_dir)
filename = f"{output_dir}/mystery.tex"
try:
with open(filename, "w", encoding='utf-8') as f:
f.write(tex)
except OSError as e:
raise RuntimeError(f"Failed to save LaTeX file: {e}") from e
🧰 Tools
🪛 Ruff

47-47: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will be solved later. Please add a comment about it in #15

Copy link

Choose a reason for hiding this comment

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

@neuromancer, I've added a comment to issue #15 as requested.

🔗 Analysis chain

@neuromancer, I've added a comment to issue #15 as requested.


Apologies for the error in the script. The backticks around save_tex() were causing the syntax issue. Here's the corrected script:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Add a comment to issue #15 noting that the exception chaining will be addressed there.
gh issue comment 15 --body "$(cat <<EOF
Noted that the exception chaining in the `save_tex()` function will be addressed in this issue, as per [PR #13](https://github.com/mystery-o-matic/mystery-o-matic.github.io/pull/13) and [this discussion](https://github.com/mystery-o-matic/mystery-o-matic.github.io/pull/13#discussion_r1833874647).
EOF
)"

Length of output: 525


Script:

#!/bin/bash
# Add a comment to issue #15 noting that the exception chaining will be addressed there.

gh issue comment 15 --body "$(cat <<'EOF'
Noted that the exception chaining in the `save_tex()` function will be addressed in this issue, as per [PR #13](https://github.com/mystery-o-matic/mystery-o-matic.github.io/pull/13) and [this discussion](https://github.com/mystery-o-matic/mystery-o-matic.github.io/pull/13#discussion_r1833874647).
EOF
)"

Length of output: 430

return filename

def get_char_name(name):
if name == "NOBODY":
return name
return name.capitalize()

def get_emoji_name(emoji_char):
# Use emoji.demojize to get the name in the format ":emoji_name:"
emoji_name = demojize(emoji_char)
emoji_name = emoji_name.strip(":").replace("_", "-")
# Strip the colons to return just the canonical name
return "\\emoji{" + emoji_name + "}"

def replace_emojis(text):
no_emojis = demojize(text).replace("(:", "(\\emoji{")
no_emojis = no_emojis.replace(":)", "})")
no_emojis = no_emojis.replace("_", "-")
return no_emojis
Comment on lines +63 to +67
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Make emoji replacement more robust.

The current implementation using string replacement is fragile and could accidentally replace non-emoji text. Consider using regex or the emoji library's functionality directly.

 def replace_emojis(text):
-    no_emojis = demojize(text).replace("(:", "(\\emoji{")
-    no_emojis = no_emojis.replace(":)", "})")
-    no_emojis = no_emojis.replace("_", "-")
-    return no_emojis
+    import re
+    # Convert emojis to their names
+    text_with_names = demojize(text)
+    # Replace emoji names with LaTeX commands
+    pattern = r':([\w-]+):'
+    return re.sub(pattern, r'\\emoji{\1}', text_with_names).replace("_", "-")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def replace_emojis(text):
no_emojis = demojize(text).replace("(:", "(\\emoji{")
no_emojis = no_emojis.replace(":)", "})")
no_emojis = no_emojis.replace("_", "-")
return no_emojis
def replace_emojis(text):
import re
# Convert emojis to their names
text_with_names = demojize(text)
# Replace emoji names with LaTeX commands
pattern = r':([\w-]+):'
return re.sub(pattern, r'\\emoji{\1}', text_with_names).replace("_", "-")


def generate_latex_clue_table(name, num_columns, num_rows, include_header = True):
# Define the LaTeX column specifier string
column_spec = '|c|c|' + '|'.join(['>{\\centering}m{0.04\\paperwidth}' for _ in range(num_columns)]) + '|'

# Start building the LaTeX table code
latex_code = "\\begin{tabular}{" + column_spec + "}\n\\hline\n"

# First row with $$ROOM0REP and times

multirow_number = num_rows
if (include_header):
multirow_number += 1

latex_code += "\\multirow{ "+ str(multirow_number) + "}{*}{ $$" + name + " } "

if (include_header):
latex_code += "& \\emoji{mantelpiece-clock} & "
# Add the $$TIME elements in the first row
time_cells = [f"$$TIME{i}" for i in range(num_columns)]
latex_code += " & ".join(time_cells) + " \\tabularnewline\n"

# Add individual clines for each column from the second to the last
for col in range(2, num_columns + 2):
latex_code += f"\\cline{{{col}-{num_columns + 2}}} "
latex_code += "\n"

# Rows with $$CHAR1, $$CHAR2, $$CHAR3
for char_num in range(1, num_rows + 1):
latex_code += f" & $$CHAR{char_num} & " + " & ".join([""] * num_columns) + " \\tabularnewline\n"

# Add individual clines for each column in the current row
if (char_num < 3):
for col in range(2, num_columns + 2):
latex_code += f"\\cline{{{col}-{num_columns + 2}}} "
latex_code += "\n"

# End the table
latex_code += "\\hline\n\\end{tabular}\\\\\n"

return latex_code

def generate_latex_weapons_table(number_weapons):
latex_code = '\\begin{tabular}{|>{\centering}p{0.172\paperwidth}|>{\centering}p{0.172\paperwidth}|>{\centering}p{0.172\paperwidth}|>{\centering}p{0.172\paperwidth}|}\n'
latex_code += "\\hline\n"
for i in range(number_weapons):
latex_code += f"$$ROOM{i}WEAPONREP & "

latex_code = latex_code[:-2] + " \\tabularnewline\n"
latex_code += "\\hline\n\\end{tabular}\n"
return latex_code
2 changes: 2 additions & 0 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
"mystery_o_matic.output",
"mystery_o_matic.output.html",
"mystery_o_matic.output.text",
"mystery_o_matic.output.latex",
],
license="AGPL3",
entry_points="""
Expand All @@ -29,6 +30,7 @@
"pygraphviz",
"networkx",
"yattag",
"emoji",
"python-telegram-bot",
],
extras_require=extra_require,
Expand Down
31 changes: 31 additions & 0 deletions static/en/mystery.template.tex
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
\subsection*{\emoji{robot}Beep boop, a new mystery is up}

$$introLocation
Help them solve this mystery checking out these initial clues:
$$initialClues

\begin{center}
\includegraphics[height=0.15\paperheight]{locations_small}
\par\end{center}

\subsubsection*{\emoji{notebook}Detective’s Journal \& Clues}
\noindent \begin{center}

$$cluesTables

$$weaponsTable

\par\end{center}
$$additionalClues
\noindent\ovalbox{\begin{minipage}[t]{1\columnwidth - 2\fboxsep - 0.8pt}%
\medskip{}

\begin{center}
\begin{tabular}{ccccc}
The murderer was... & & The weapon was... & & Time of murder was...\tabularnewline
& & & & \tabularnewline
\cline{1-1} \cline{3-3} \cline{5-5}
& & & & \tabularnewline
\end{tabular}
\par\end{center}%
\end{minipage}}
neuromancer marked this conversation as resolved.
Show resolved Hide resolved
31 changes: 31 additions & 0 deletions static/es/mystery.template.tex
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
\subsection*{\emoji{robot} beep boop, hay un nuevo misterio}

$$introLocation
Ayúdales a resolver este misterio revisando estas pistas iniciales:
$$initialClues

\begin{center}
\includegraphics[height=0.15\paperheight]{locations_small}
\par\end{center}

\subsubsection*{\emoji{notebook} Tu diario de detective y tus pistas}
\noindent \begin{center}

$$cluesTables

$$weaponsTable

\par\end{center}
$$additionalClues
\noindent\ovalbox{\begin{minipage}[t]{1\columnwidth - 2\fboxsep - 0.8pt}%
\medskip{}

\begin{center}
\begin{tabular}{ccccc}
The murderer was... & & The weapon was... & & Time of murder was...\tabularnewline
& & & & \tabularnewline
\cline{1-1} \cline{3-3} \cline{5-5}
& & & & \tabularnewline
\end{tabular}
\par\end{center}%
\end{minipage}}
Comment on lines +20 to +31
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance the solution box layout and usability.

The current table structure has several potential issues:

  1. Fixed width calculation might not work well with different paper sizes
  2. Table cells lack sufficient space for writing answers
  3. Table alignment could be improved
-\noindent\ovalbox{\begin{minipage}[t]{1\columnwidth - 2\fboxsep - 0.8pt}%
+\noindent\ovalbox{\begin{minipage}[t]{\dimexpr\linewidth-2\fboxsep-2\fboxrule\relax}%
 \medskip{}
 
 \begin{center}
-\begin{tabular}{ccccc}
-The murderer was... &  & The weapon was... &  & Time of murder was...\tabularnewline
- &  &  &  & \tabularnewline
-\cline{1-1} \cline{3-3} \cline{5-5}
- &  &  &  & \tabularnewline
+\begin{tabular}{p{0.3\linewidth}@{\hspace{1em}}p{0.3\linewidth}@{\hspace{1em}}p{0.3\linewidth}}
+\textbf{The murderer was...} & \textbf{The weapon was...} & \textbf{Time of murder was...}\tabularnewline[1em]
+\cline{1-1}\cline{2-2}\cline{3-3}
+\rule{0pt}{3em} & \rule{0pt}{3em} & \rule{0pt}{3em}\tabularnewline
 \end{tabular}
 \par\end{center}%
 \end{minipage}}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
\noindent\ovalbox{\begin{minipage}[t]{1\columnwidth - 2\fboxsep - 0.8pt}%
\medskip{}
\begin{center}
\begin{tabular}{ccccc}
The murderer was... & & The weapon was... & & Time of murder was...\tabularnewline
& & & & \tabularnewline
\cline{1-1} \cline{3-3} \cline{5-5}
& & & & \tabularnewline
\end{tabular}
\par\end{center}%
\end{minipage}}
\noindent\ovalbox{\begin{minipage}[t]{\dimexpr\linewidth-2\fboxsep-2\fboxrule\relax}%
\medskip{}
\begin{center}
\begin{tabular}{p{0.3\linewidth}@{\hspace{1em}}p{0.3\linewidth}@{\hspace{1em}}p{0.3\linewidth}}
\textbf{The murderer was...} & \textbf{The weapon was...} & \textbf{Time of murder was...}\tabularnewline[1em]
\cline{1-1}\cline{2-2}\cline{3-3}
\rule{0pt}{3em} & \rule{0pt}{3em} & \rule{0pt}{3em}\tabularnewline
\end{tabular}
\par\end{center}%
\end{minipage}}