Skip to content

Conversation

@vedant-vijay
Copy link

@vedant-vijay vedant-vijay commented Oct 27, 2025

Fixes #2961

Description

This PR fixes the language filter on the SDK page where C, C++, and C# languages could not be filtered independently. The issue occurred because the slugify filter was converting all three language names to the same value ("c"), causing class name collisions.

Problem

On the SDK page at /ecosystem/sdks/, when filtering languages, the C-family languages (C, C++, C#) weren't filtering independently. Selecting one would show SDKs for all C-family languages because their slugified class names were colliding (all becoming "c").

Solution

  • Created a reusable normalize_language() macro in a separate file templates/macros/language_helpers.html
  • Implemented proper language slug mappings:
    • C++ → cplusplus
    • C# → csharp
    • C → c
  • Used descriptive language_slug variable name throughout
  • Applied the normalization consistently to both filter checkboxes and SDK card classes
  • Eliminated code duplication by using a centralized macro

Test Steps

  1. Navigate to the SDKs page at /ecosystem/sdks/
  2. Click the "Language" drop-down and deselect all languages
  3. Select only "C++" - verify only C++ SDKs are displayed
  4. Add "C#" to the selection - verify both C++ and C# SDKs are displayed
  5. Deselect C++ and select only "C" - verify only C SDKs are displayed
  6. Try various combinations - each should show exactly the selected languages with no cross-contamination

Files Changed

  • Created: templates/macros/language_helpers.html - Contains the normalize_language macro for consistent language slug generation
  • Modified: templates/macros/sdks.html - Imports and uses the macro for language filtering in both filter menu and SDK cards

Review Changes Addressed

All feedback from @HarHarLinks has been implemented:

  • ✅ Extracted repeated code into a reusable macro (separate file for cross-template accessibility)
  • ✅ Changed C++ slug from plusplus to cplusplus
  • ✅ Kept C# as csharp (no mixing of language/platform naming)
  • ✅ Changed C slug from clanguage to c
  • ✅ Used descriptive language_slug variable name
  • ✅ Updated PR description to follow the template

Checklist

Additional Notes

Tested locally with zola serve - all filters work correctly with no errors.

Copy link
Collaborator

@HarHarLinks HarHarLinks left a comment

Choose a reason for hiding this comment

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

Thanks, this seems to work well. Just some remarks on making it a bit prettier :)

@HarHarLinks
Copy link
Collaborator

Please read, apply, and keep in your PR description the PR template https://github.com/matrix-org/matrix.org/blob/main/.github/pull_request_template.md, including e.g. how to link to relevant issues.

@HarHarLinks HarHarLinks added bug Something is broken. ecosystem Adding and removing ecosystem projects labels Oct 28, 2025
- Extract normalize_language into separate macro file
- Change C++ slug from 'plusplus' to 'cplusplus'
- Change C slug from 'clanguage' to 'c'
- Use language_slug variable name for clarity
@vedant-vijay
Copy link
Author

Changes Requested - All Addressed ✅

Thank you @HarHarLinks for the detailed review! I've implemented all the requested changes:

What Changed:

  1. Extracted repeated code into a macro

    • Created templates/macros/language_helpers.html with normalize_language macro
    • Both filter menu and SDK cards now use this shared macro
  2. Updated language slug names per your suggestions:

    • C++ → cplusplus (changed from plusplus)
    • C# → csharp (no change needed)
    • C → c (changed from clanguage)
  3. Used descriptive variable naming

    • Kept language_slug throughout
  4. Updated PR description

Files Modified:

  • Created: templates/macros/language_helpers.html
  • Modified: templates/macros/sdks.html

Testing:

  • zola serve runs without errors
  • ✅ Language filters work independently for C, C++, and C#
  • ✅ No class name conflicts

Copy link
Collaborator

@HarHarLinks HarHarLinks left a comment

Choose a reason for hiding this comment

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

Thank you for addressing the feedback. Functionally this PR now seems in a good place. Some style issues remain.

What sticks out to me however are the rather extremely verbose comments which consistently include some typical errors. I don't want to accuse anyone wrongly, but I am currently very convinced that this is an LLM-generated PR. It also seems obvious that the PR template was not in fact read, as it specifically states some things to do or not do when contributing, that were apparently ignored.

As a maintainer, I am interested in good contributions to this repo, and I am interested in teaching about writing code and contributing to open source in general. I am not interested in spending my time explaining things to a proxy for an LLM, or reviewing LLM contributions in general, let alone burning own my planet. If you want to learn to code, I can only strongly suggest to actually learn things, and in my experience the best way to do that is to do it yourself, fail, and try again until you got it.

@vedant-vijay
Copy link
Author

vedant-vijay commented Oct 30, 2025

Thank you, @HarHarLinks, for the honest feedback, which I take as advice and guidance from a senior coder. This is my first ever PR, and I am just starting my open source journey. I want to keep it more professional for the maintainers. This is the reason I used LLM for writing a complete PR description but that is my mistake for not being transparent about it.

Regarding the technical implementation, I want to clarify that I solved the core bug by myself by adding the if-else logic in the code, but after your feedback on code reusability and using macros instead, I implemented it in the same file, sdk.html. But I encountered the unexpected problem because of the sdk_deck.html. The error is about Tera, when I define a macro inside sdk.html, both macros were in sdks.html, but normalize_language() is not imported into sdk_deck.html's namespace. Even though both macros are in the same file sdks.html, when sdks_panel is called from another file, it loses access to sibling macros defined in the same file. So I went on ChatGPT to explore other approaches to debugging this error, and I got the idea of creating a separate file (language_helper.html) to make the language conversion function available to any file. I didn't directly use LLM to solve this bug. I tried first and was able to debug using my own logic, also learned about code reusability, Tera, a legitimate technical challenge I solved, and updated it using an LLM approach. I think it's a good use of LLM, which helps me to gain more knowledge about the other approach to the bug. I think its good coding practice, and I used AI and LLM not to write the complete code but to understand the problem and explore more possibilities, needs your guidance and suggestions on this take, sir.

Even if these commits are not accepted by the maintainers, I am more than happy because, as my first PR, I got to learn about so many new practices for writing code in just one PR. I still apologise for using AI without consent, sir. Thank you.

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

Labels

bug Something is broken. ecosystem Adding and removing ecosystem projects

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Ecosystems/SDKs - Filter by language cannot distinguish between C, C#, and C++

2 participants