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

I reviewed my old changes and merged them into the current branch #79

Open
wants to merge 23 commits into
base: beta
Choose a base branch
from

Conversation

LordMartron94
Copy link

I primarily refactored the wildcard behavior and made a temporary fix for the path too long issues...

⚠️⚠️⚠️ While everything works and doesn't crash, I haven't yet merged the custom wildcard stuff you made in the meantime into it. ⚠️⚠️⚠️

I will let you know once that is done.

I will also look for a way to make the path length thing more robust.

Lastly, I am looking to becoming more active in the maintaining and development of this project once again. Most notably with refactoring stuff to be easier to maintain. I might also attempt to add new features and solve bugs.

@LordMartron94
Copy link
Author

https://forums.zotero.org/discussion/121226/fix-file-length-limit-ioutils?new=1

Asked for a way to configure path length limits so we can fix this in a more robust way.

@wileyyugioh
Copy link
Owner

Wow! This is a massive pull request so please give me some time to review.

One thing I noticed while skimming is that you changed var X = class to class X. The reason I did var X = class is because reloading the js files (via disabling/enabling the addon) causes errors due to re-declaring the class. Does this happen in the pr?

@LordMartron94
Copy link
Author

Wow! This is a massive pull request so please give me some time to review.

One thing I noticed while skimming is that you changed var X = class to class X. The reason I did var X = class is because reloading the js files (via disabling/enabling the addon) causes errors due to re-declaring the class. Does this happen in the pr?

Take your time. I haven't checked that as I didn't have that issue before, but I'll be sure to test that once I get back.

Again, I also still need to incorporate the custom wildcard behavior you added in between my old fixes and now. It shouldn't be too difficult to merge those in though.

@LordMartron94
Copy link
Author

Ahh I see you are working on it yourself, awesome.

Apologies, have been a bit busy myself.

The check for first load is smart too to resolve the issue you encountered :)

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