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

"Unknown" when code is missing. #558

Open
pulgamecanica opened this issue Oct 5, 2022 · 9 comments
Open

"Unknown" when code is missing. #558

pulgamecanica opened this issue Oct 5, 2022 · 9 comments
Labels
Experience: Backend hacktoberfest Good issue for someone to work on for Hacktoberfest help wanted We'd love to have help working on this issue Type: Enhancement

Comments

@pulgamecanica
Copy link
Contributor

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:

def concept_code(self, concept_key):

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 :

def format_code_for_display(concept_key, lang):

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

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
Copy link
Member

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
Copy link

github-actions bot commented Oct 31, 2022

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
Copy link
Member

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

@geekygirlsarah
Copy link
Member

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
Copy link
Contributor Author

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.

  • Make the code always mandatory when the concept is valid regardless if there is comment or not.
  • Make the code mandatory when the concept is valid and there are no comments, if comment is present, should be valid?

@github-actions
Copy link

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 [email protected].If you are still working on this issue, that's fine. Please comment here to tell the bot to give you more time.

@geekygirlsarah
Copy link
Member

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
Copy link

github-actions bot commented Dec 3, 2022

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 [email protected].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
Copy link

github-actions bot commented Dec 6, 2022

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.

@geekygirlsarah geekygirlsarah added Type: Enhancement help wanted We'd love to have help working on this issue hacktoberfest Good issue for someone to work on for Hacktoberfest Experience: Backend labels Oct 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Experience: Backend hacktoberfest Good issue for someone to work on for Hacktoberfest help wanted We'd love to have help working on this issue Type: Enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants