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

Reload function #136

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Reload function #136

wants to merge 4 commits into from

Conversation

TerminalMan
Copy link

Added board reload functionality. 'reload currently opened file'
File -> Reload
CTRL+r

  • some minor necessary changes.

@inflex
Copy link
Member

inflex commented Jul 16, 2018

I can understand this being used in a custom fork of OBV for developments but unsure if it's something that is applicable in the main release, as I understand it you're using it so you can quickly check on your boardview file generation ?

If it's felt that this feature should be added, I'd say it may be better to modify the existing Load() call such that if it looks for a flag to preserves the state rather than the separate loader. That way we don't have to remember to duplicate code for any new file format being added in to the Load call in the future.

…code. LoadFile() now has a flag that lets you reset or keep the view.
@TerminalMan
Copy link
Author

Yes, the feature is mainly meant to make the development of exporters easier. But seeing that it isn't a difficult to understand feature I thought that adding it to master wouldn't hurt.

That being said, I already refactored the code so that ReLoadFile() is now integrated into LoadFile(), to address your 2nd point.

The view reset can now be triggered via a LoadFile() bool parameter.

@MattSturgeon
Copy link
Contributor

I think it's best to avoid uneccessary feature bloat, however if something is especially helpful for development it may be worth adding it behind a build flag - so long as it doesn't get in the way of the main codebase and the added complexity is worth the extra convenience.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants