Added Recent File Menu #209
Conversation
OpenMotor now tracks recently opened files and lets you easily return to them.
|
|
||
| def addRecentFile(self, filepath): | ||
| recentFilepaths = [] | ||
| too_long = False |
uilib/fileManager.py
Outdated
| @@ -24,6 +25,10 @@ def __init__(self, app): | |||
| self.fileName = None | |||
|
|
|||
| self.newFile() | |||
|
|
|||
| self.recentlyOpenedFiles = [] | |||
| self.recentFiles = getConfigPath() + 'recentfiles.txt' | |||
There was a problem hiding this comment.
It is a pretty minor difference here, but I'd prefer the use of a yaml file with the same fileIO setup I used for preferences. That is consistent with the rest of the application and gets us things like migrations if we ever need them. You should just be able to stick a list in one of them.
There was a problem hiding this comment.
Another thing: concatenating file paths like that isn't always good practice because different operating systems might have different schemes for it. The safest way is with os.path.join. I know I used the + for this in the application myself, but I have learned since then!
OpenMotor now tracks recently opened files and lets you easily return to them.
…/openMotor into recently_opened_files
|
Hi Andrew, |
There was a problem hiding this comment.
Thanks for coming back to this, much closer! Couple of minor comments.
I pulled the branch down and played with it for a bit and liked it! I did find one bug, though. I noticed that it doesn't update the propellant selector when you load a motor using the recent files menu. After digging around for a bit, it seems like it is because of a confusing interaction between the FileManager and the main window. Basically, in other places that motors are loaded, the main window calls the fileManager.load method, then, if a motor was loaded, a function calls postLoadUpdate, which among other things, disables events on the prop selector and changes what it is pointing to, After calling postLoadUpdate, it then calls enablePropSelector. This is pretty confusing and probably should be refactored, but is out of scope for this PR.
Instead, the quick fix here is to pass in the main window instance to each RecentFile object we instantiate instead of the file manager, and call the loadMotor on that instead of load. I think that should "just work".
| self.recentlyOpenedFiles = [] | ||
|
|
||
| self.recentFilesList = loadFile(self.recentFiles, fileTypes.RECENTFILES)["recentfileslist"] | ||
| if len(self.recentFilesList) >= 1: |
There was a problem hiding this comment.
Nitpick: This if is redundant, as looping over an empty array does nothing.
|
|
||
| def updateRecentlyOpenedMenu(self): | ||
| self.recentlyOpenedMenu.clear() | ||
| self.recentlyOpenedFiles = [] |
There was a problem hiding this comment.
Nit: please name this something that indicates that it contains the menu actions. I was briefly confused about what the difference between self.recentlyOpenedFiles and self.recentFilesList was.
|
|
||
| def addRecentFile(self, filepath): | ||
| recentFilepaths = [] | ||
| too_long = False |
OpenMotor now tracks recently opened files and lets you easily return to them.