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

Loading of custom docs from txt files #155

Merged
merged 4 commits into from
Sep 16, 2024

Conversation

BrokenGravityMusic
Copy link
Contributor

@BrokenGravityMusic BrokenGravityMusic commented Sep 14, 2024

Feature: Adds the ability to load custom documentation for effects.

  • Looks for documentation file <settingsDir>/customDocs/<plugin-name>.txt
  • If custom documentation exists, it is prepended to the standard documentation with a separator.
  • The last loaded custom documentation is cached to avoid repeated file reads on calls to resizeDocArea()

(settingsDir is the same dir where favorites are stored)

Code changes:

  • Added loadCustomDocumentation() to handle reading and caching documentation.
  • Updated resizeDocArea() to include custom documentation if available.
  • Move directory stuff from getFavoritesFile(), into its own getSettingsDirectory(), for reuse in loadCustomDocumentation()

@baconpaul
Copy link
Owner

Oh this is awesome
We have some visitors here now but I will review tomorrow morning. One thing we may want to add is an entry to the doc and perhaps also a menu item to open custom text folder, but that’s just at a first glance

@baconpaul
Copy link
Owner

That compile error is real it seems. You don’t need that qualifier in the header

@BrokenGravityMusic
Copy link
Contributor Author

I pushed a change. Sorry I'm new to this, but i don't see that output with the error from before. Does that mean it passed?

@BrokenGravityMusic
Copy link
Contributor Author

Also i have (work in progress) generated docs here that can be used for testing.
Since the patch only looks for *.txt, this repo can be checked out directly as C:\Users\username\Documents\Airwindows\customDocs

Works fine for me (famous last words). :)

@baconpaul
Copy link
Owner

I pushed a change. Sorry I'm new to this, but i don't see that output with the error from before. Does that mean it passed?

since you don't have a merge on this repo yet i need to approve each run. Once i merge your change (which i suspect I will do very soon) it will auto run if you submit a PR.

@baconpaul
Copy link
Owner

OK @BrokenGravityMusic I'm going to merge this. I think there's a couple more things which would be nice which you could send in a separate PR if you dont' mind

  1. An update to the manual in the doc/ folder with this
  2. A menu item called "Show custom docs dir" in settings which does an open directory type thing.

If you need help with either of those just ask but I think this is already useful.

@baconpaul baconpaul merged commit b23583d into baconpaul:main Sep 16, 2024
11 checks passed
@BrokenGravityMusic BrokenGravityMusic deleted the customdocs branch September 17, 2024 18:12
@BrokenGravityMusic
Copy link
Contributor Author

2. A menu item called "Show custom docs dir" in settings which does an open directory type thing.

Maybe this should be "Show config dir?", since customDocs is just a dir in config/settings dir, and if there is ever any other sub-dir people might be interested in going to, we wouldn't need 2 of these menu options. Or maybe that's premature optimization. What do you think?

Is this the place to chat about this? Or is there some better place?

@baconpaul
Copy link
Owner

This is a fine place to chat or feel free to open another issue here or if you are a discord user the airwindows channel of surge discord also works.

And yeah I should have said '2. a menu item called something like show blah which does something like blah'. Whatever the appropriate gesture and organization is. Sounds like config dir is correct indeed.

@BrokenGravityMusic BrokenGravityMusic restored the customdocs branch September 18, 2024 05:36
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.

2 participants