Have enabling/disabling debugging not require restarting Atom

dmoonfire commented 3 years ago

Requirements

Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
All new code requires tests to ensure against regressions

Description of the Change

When the debug function is disabled, it create a () => {} function to swallow logging. However, if the debug flag is switched on, the saved log properties retain the empty function and don't start logging until Atom is restarted.

Likewise, when the debugging is disabled keeps the debug versions of the code and they continue to operate.

In addition, localStorage.debug needed to be set for debugging to work. Since we have a configuration values, the change inserts the required local storage keys for useful debug operations.

Alternate Designs

Using the current implementation works, it just requires extra steps.

Benefits

Enabling and disabling debug should control the logging immediately, this makes it a less effort and easier to work with.

Possible Drawbacks

The handle the wrapped version of the logger requires extra complexity. There might be some edge cases and it will have a small impact on startup time to load the additional module.

Applicable Issues

This extends on https://github.com/atom/spell-check/pull/357.

dmoonfire commented 3 years ago

@sadick254 would you be willing to look at this and see if it works with what you were trying to implement with your item? Thank you.

dmoonfire commented 3 years ago

@sadick254 related to this, I have a question. According to https://github.com/atom/spell-check/issues/356, "It should not be loaded in the production code" means it won't ever load in the release?

If that is the case, then I might need to come up with an alternative since I added debug specifically to turn on logging to debug production spell-checking errors and have users be able to copy/paste the output so I can investigate the issues that I can't duplicate myself (such as on Macs). I would consider that a "production" use.

9994444ggg commented 3 years ago

Thank You Back dont help me more for the moment please.

sadick254 commented 3 years ago

@dmoonfire I now have this on my radar. I will look into it. Thanks for your contribution. 👍

dmoonfire commented 2 years ago

@sadick254, I've just looked at this again and I believe it will solve the problem with https://github.com/atom/spell-check/issues/372 in a more generic manner (there is a missing const in the current, but this PR pulls the constructor into a different file).

If you don't see any problems, I think it would solve a problem a number of people have had, but I'm not sure what's happened. I can also merge it myself, but I typically request one other person review it before I do that (never trust one's own code).

dmoonfire commented 2 years ago

This also would fix #369.

sadick254 commented 2 years ago

@dmoonfire. Thanks for opening a PR. I will take a look into it.

atom / spell-check