codethesaurus / codethesaur.us

A polyglot developer reference tool to compare programming language concepts side-by-side! Great for learning new languages or using for reference.
https://codethesaur.us
GNU Affero General Public License v3.0
289 stars 170 forks source link

"Unknown" when code is missing. #558

Open pulgamecanica opened 2 years ago

pulgamecanica commented 2 years ago

Description

It will remove the "Unknown" message which is shown on the reference page if the concept is VALID but it has no "code" text. I agree, "Unknown" should be displayed, but I think only when the concept is not valid... not when there is no "code", because there might be a "comment" that can explain a concept, and NO "code" at all.

Explanatory image:

Screen Shot 2022-10-05 at 11 19 48

In the example, the default string byte encoding has a comment which explains the concept, but no "code" is provided.

"default_string_byte_encoding": {
    "name": "Default byte encoding (ex: ASCII UTF-8, UTF-16, etc.)",
     "comment": "UTF-8"
}

You can see on the image, that "Unknown" is displayed above the comment.

Requirements

Whenever the JSON file has a valid concept, it should consider that it MAY include "code" or "comment" or both at the same time.


On the models, on this function:

https://github.com/codethesaurus/codethesaur.us/blob/229b52766c32ed2fbf8edcbe03dcc06460d650e6/web/models.py#L134

I would change it to something like this:

def concept_code(self, concept_key):
        code = self.concept(concept_key).get("code")
        if not code:
            return None
        if isinstance(code, list):
            code = "\n".join(code)
        return code

Ensuring that code returns None, when it's not present on the JSON file


On the views on this function :

https://github.com/codethesaurus/codethesaur.us/blob/229b52766c32ed2fbf8edcbe03dcc06460d650e6/web/views.py#L376

I would change it to something like this:

def format_code_for_display(concept_key, lang):
    if lang.concept_unknown(concept_key):
        return "Unknown"
    if lang.concept_code(concept_key) is None:
        return None
    if lang.concept_implemented(concept_key):
        return highlight(
            lang.concept_code(concept_key),
            get_lexer_by_name(lang.key, startinline=True),
            HtmlFormatter()
        )
    return None

So, if the concept is defined but the code is None, then you return None, instead of "Unknown"


This way, the template will not create the div for the "code" section and the condition will make more sense

https://github.com/codethesaurus/codethesaur.us/blob/229b52766c32ed2fbf8edcbe03dcc06460d650e6/web/templates/concept_card.html#L5

Additional Notes

In the end, it could look something like this...

Screen Shot 2022-10-05 at 15 40 12

For the given code:

Screen Shot 2022-10-05 at 15 41 18

The "Unkown" is gone because the concept was well defined even when the "code" is not present (there is "comment")

Cheers, I would love to hear some feedback about this ;)

geekygirlsarah commented 2 years ago

Hmm... this is a good point. It's a weird edge case because there's: 1) concept exists (therefore has example code) 2) concept doesn't exist (therefore can't have code) 3) unknown state (the ID got added but wasn't added to the file)

and to distinguish between 2 and 3, we added "not-implemented" as a way to flag the system that the concept doesn't exist. I think it even allows a comment with it too.

But this is a another case, a case 4. Where it is valid (#1) but code is empty/blank. And maybe that's a good point, it should just be empty and post a comment, though I think the original thought is "code" is required and "comment" is optional. So if they drop "code", the idea is it goes against the ideal syntax of the file and how it should work.

Let me brew on this a bit more. I think you're probably right something needs to happen, I'm just pondering different answers at the moment. 🤔

github-actions[bot] commented 2 years ago

This issue has been inactive for 346 hours (14.42 days) and will be unassigned after 62 more hours (2.58 days). If you have questions, please leave a comment or let @geekygirlsarah know on Twitter or by email.

If you are still working on this issue, that's fine. Please comment here to tell the bot to give you more time.

geekygirlsarah commented 2 years ago

Leaving a comment as I said I'd return to this and haven't yet.

geekygirlsarah commented 2 years ago

I guess this is fine to leave in, however, I'd like the validatelanginfofiles command and GitHub Action to reflect this. Would you mind updating that too? It's in web/management/commands/validatelanginfofiles.py

pulgamecanica commented 2 years ago

Right now I am unable to code, because I don't have a computer with me, I had to travel to Mexico because my grandma died, so I've been mourning her. I do have one question though so I can think about this subject:

In which way should it be changed?

We have proposed a few ideas.

github-actions[bot] commented 1 year ago

This issue has been inactive for 341 hours (14.21 days) and will be unassigned after 67 more hours (2.79 days). If you have questions, please leave a comment, message @codethesaurus or @geekygirlsarah on Twitter, or email coreteam@codethesaur.us.If you are still working on this issue, that's fine. Please comment here to tell the bot to give you more time.

geekygirlsarah commented 1 year ago

Commenting to leave this open.

Let's go ahead with it. If code or comment is there, it's not "unknown". One of them has to exist and can't be empty (empty string).

github-actions[bot] commented 1 year ago

This issue has been inactive for 347 hours (14.46 days) and will be unassigned after 61 more hours (2.54 days). If you have questions, please leave a comment, message @codethesaurus or @geekygirlsarah on Twitter, or email coreteam@codethesaur.us.If you are still working on this issue, that's fine. Please comment here to tell the bot to give you more time.

github-actions[bot] commented 1 year ago

This issue has been inactive for 408 hours (17.00 days) and is past the limit of 408 hours (17.00 days) so is being unassigned.You’ve just been unassigned from this ticket due to inactivity – but feel free to pick it back up (or a new one!) in the future! Thank you for your interest in contributing to this project.