Details

    • Type: Sub-task
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0
    • Fix Version/s: 2.0
    • Component/s: Libraries
    • Labels:
      None
    • Affected Branches:
      MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE

      Description

      It appears to be useful to allow an extra word in the plugin renderer class name, like moodle_mod_workshop_subthing_renderer - so you can have multiple renderers belonging to one plugin.

      See http://moodle.org/mod/cvsadmin/view.php?conversationid=2705 for the background

        Gliffy Diagrams

          Activity

          Hide
          mudrd8mz David Mudrak added a comment -

          Fixed URL

          Show
          mudrd8mz David Mudrak added a comment - Fixed URL
          Hide
          mudrd8mz David Mudrak added a comment -

          See the attached 0001 patch for the proposed implementation

          If you are using git, you can run
          git am <<the patch filename>>
          or - if it fails - try
          git am -3 <<the patch filename>>

          Non-gitters will just remove some lines from the top of the patch file to get pure diff

          Show
          mudrd8mz David Mudrak added a comment - See the attached 0001 patch for the proposed implementation If you are using git, you can run git am <<the patch filename>> or - if it fails - try git am -3 <<the patch filename>> Non-gitters will just remove some lines from the top of the patch file to get pure diff
          Hide
          timhunt Tim Hunt added a comment -

          That looks good.

          I assume you have run the unit tests, and they all pass. Please comment. Thanks.

          Show
          timhunt Tim Hunt added a comment - That looks good. I assume you have run the unit tests, and they all pass. Please comment. Thanks.
          Hide
          mudrd8mz David Mudrak added a comment -

          Yes, the tests passed. I modified some of the current tests a bit - I know I should not do that but I felt I am doing right thing, hopefully. Also, I added some more to test the variations of calls with the new parameter. The new parameter works for my needs.
          To be reviewed more carefully: how I integrated this into the templateable renderers as I am not very confident in this area. Thanks in advance.

          Show
          mudrd8mz David Mudrak added a comment - Yes, the tests passed. I modified some of the current tests a bit - I know I should not do that but I felt I am doing right thing, hopefully. Also, I added some more to test the variations of calls with the new parameter. The new parameter works for my needs. To be reviewed more carefully: how I integrated this into the templateable renderers as I am not very confident in this area. Thanks in advance.
          Hide
          timhunt Tim Hunt added a comment -

          Doh! that was an interesting typo. I mean to say Please commit.

          However, you are right template_renderer_factory needs a bit more work. In get_renderer, the $searchpaths array needs to

          $path = $rootpath . '/' . $module;
          if (!is_null($subtype))

          { $path .= '/' . $subtype; }

          if (is_dir($path))

          { $searchpaths[] = $path; }

          Is probably a good way to do it.

          Anyway, template renderer is only an experimental proof-of-concept. No need to be too concerned with it now.

          Show
          timhunt Tim Hunt added a comment - Doh! that was an interesting typo. I mean to say Please commit. However, you are right template_renderer_factory needs a bit more work. In get_renderer, the $searchpaths array needs to $path = $rootpath . '/' . $module; if (!is_null($subtype)) { $path .= '/' . $subtype; } if (is_dir($path)) { $searchpaths[] = $path; } Is probably a good way to do it. Anyway, template renderer is only an experimental proof-of-concept. No need to be too concerned with it now.
          Hide
          mudrd8mz David Mudrak added a comment -

          Well, this was my original intention to keep all templates in one directory. But you are right, subdirectories templates should respect the subtypes.

          p.s. I like the typo. Maybe we can start using "Fix a lot" instead "Thanks a lot" and so on

          Show
          mudrd8mz David Mudrak added a comment - Well, this was my original intention to keep all templates in one directory. But you are right, subdirectories templates should respect the subtypes. p.s. I like the typo. Maybe we can start using "Fix a lot" instead "Thanks a lot" and so on
          Hide
          mudrd8mz David Mudrak added a comment -

          Committed. I have fixed the template_renderer_factory::get_renderer() so it sets up folders for subtypes. This involved a little extension of the unit test.

          Show
          mudrd8mz David Mudrak added a comment - Committed. I have fixed the template_renderer_factory::get_renderer() so it sets up folders for subtypes. This involved a little extension of the unit test.
          Hide
          timhunt Tim Hunt added a comment -

          Great. Thanks.

          Show
          timhunt Tim Hunt added a comment - Great. Thanks.

            People

            • Assignee:
              mudrd8mz David Mudrak
              Reporter:
              mudrd8mz David Mudrak
              Tester:
              Tim Hunt
              Participants:
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:
                Fix Release Date:
                24/Nov/10