Message297462
Thanks for pointing out mapping protocol access. That's what I was (poorly) trying to describe in the __getitem__ docstring in v1. It's a really concept.
I worked on the tests. I added some more code to the classes, so I uploaded that too (sorry, but I needed help understanding the functions).
Testing your Page class seemed pretty easy, so hopefully I did it right. I have added more methods to the class though, which I didn't add tests for yet, in case it's not the right direction.
I changed the existing test to use the Changes class. I believe I did what you intended, but I didn't expand it to keys and highlights just in case.
Now, my difficulty. I had trouble trying to figure out the test for 'save_as', so I wanted to show you what I had so far. You don't need to fill it out; I just needed to see if I was on the right track. Writing a test without code was difficult since it involved other classes. So, I copied the existing code into the class to see how it would fit. Observations:
1. The `testcfg` was a IdleUserConfParser dictionary. I made a dummy config parser to override Save instead of mocking it. I think you prefer this way of doing that.
2. Page needed to know it's 'config_type' so that it (or Changes) could reach into idleConf. self.main on its own didn't know which part of the config files to update.
3. Following #2, I created a 'save' module within Page. My options were idleConf.userCfg[page.name] (in Changes) or idleConf.userCfg[self.name] (in Page). Based on your LoD comment, I thought Page should know about that and not Changes.
4. Split out `save_as` into another method called `changed`, also in Page. I thought it made sense for a Page to say if it's changed.
5. set_user_value is the same as before, although I changed the docstring and it's in Page for the page.name vs self.name reason. However, this still looks really clunky to me.
Questions:
1. Any interest on making Page inherit from defaultdict to simplify additem?
2. I'm not sure about the name for Page, but I haven't come up with an alternate. My reason is that in configdialog, all the create functions are create_page_* and the names on the tabs don't match the config (eg, there's a General tab stored in main). Unless you made that connection on purpose? I was thinking of `page` more as a GUI item, maybe because of terms like webpage. Maybe the solution is to make the functions create_tab_*?
I still haven't looked at extensions. I should have more time tomorrow.
One note, I found a typo in the current test file. In tearDownModule, it sets idleConf.userCfg to testcfg, but I think the intent was to set it back to the saved value, which is userCfg.
Thanks! |
|
Date |
User |
Action |
Args |
2017-07-01 01:18:00 | cheryl.sabella | set | recipients:
+ cheryl.sabella, terry.reedy |
2017-07-01 01:18:00 | cheryl.sabella | set | messageid: <1498871880.09.0.742414411059.issue30779@psf.upfronthosting.co.za> |
2017-07-01 01:18:00 | cheryl.sabella | link | issue30779 messages |
2017-07-01 01:17:58 | cheryl.sabella | create | |
|