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

Directive with options should not used include list of options with values such as True or False #114

Open
jaguarfi opened this issue Dec 16, 2020 · 1 comment
Labels
area: questionnaire Related to the questionnaire directive effort: days Solving this issue could take days. Starting with the creation of a new branch to the merging experience: moderate required knowledge estimate priority: low type: feature New feature or change to a feature

Comments

@jaguarfi
Copy link
Contributor

The questionnaires have some options that require the user to specify whether the value for some options is either True or False. However, If we really want to follow the syntax of reStructuredText and/or Sphinx, we must evaluate whether the options are added or not, and consequently convert the inclusion of those options to a True or False value.

The following example include the :show-model: and :reveal-model-at-max-submissions: options. Booth require a boolean value.

. questionnaire:: override-settings 40
  :title: Questionnaire with both options. (``:show-model:`` & ``:reveal-model-at-max-submissions:``)
  :submissions: 1
  :show-model: True
  :reveal-model-at-max-submissions: True
...

One example of the behaviour we should follow can be seen in the toctree directive. In the toctree directive If the user wants to hide the toctree, he/she only has to add the :hidden: option (True value), but if the user wants to see the toctree he/she do not need to add the option at all (False value)

.. toctree::
   :hidden:

   doc_1
   doc_2

We have some other directives in a-plus-rst-tools that have the same logic as the toctree, but for some reason the :show-model: and :reveal-model-at-max-submissions: options and some others have this weird behaviour.

@jaguarfi jaguarfi added type: feature New feature or change to a feature priority: low area: questionnaire Related to the questionnaire directive experience: moderate required knowledge estimate effort: days Solving this issue could take days. Starting with the creation of a new branch to the merging labels Dec 16, 2020
@markkuriekkinen
Copy link
Contributor

It was necessary to explicitly specify the True/False value so that the directive can override the default value set for the course. The default is either True or False depending on the course settings.

I don't remember this exactly, but there may also be a difference of defining the value or not defining it. If it is not defined, then it might not save the field in the config.yaml at all.

There was a reason for doing it this way, so it is not "just weird behaviour". Other implementations did not work correctly. The backwards-compatibility is also a major factor here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: questionnaire Related to the questionnaire directive effort: days Solving this issue could take days. Starting with the creation of a new branch to the merging experience: moderate required knowledge estimate priority: low type: feature New feature or change to a feature
Projects
None yet
Development

No branches or pull requests

2 participants