Repository: spyder-ide/qtsass

Issue:
Improve logging configuration
Currently qtsass is using basicConfig to configure the root logger. This is generally okay when qtsass is used only from the CLI. When used as a library this is problematic, and can propagate to other library's loggers. We should use logging best practices for a library:

```
# Add a null handler
import logging
logging.getLogger('qtsass').addHandler(logging.NullHandler())
```

We can add an enable_logging function to the api that accepts an optional logging level.

```
def enable_logging(level=None):
    if level is None:
        level = os.environ.get('QTSASS_DEBUG', logging.WARNING)
    logger = logging.getLogger('qtsass')
    logger.setLevel(level)
```

A --debug flag can be added to the cli to allow users to enable debug logging on the cli.


With that users would have the following options to configure logging:

1. Set the QTSASS_DEBUG environment variable
2. Use the --debug cli flag
3. Call qtsass.enable_logging(logging.DEBUG)
3. Add custom logging handlers to the qtsass logger

Discussion:

Patch:
diff --git a/README.md b/README.md
index 50d1d04..03048b6 100644
--- a/README.md
+++ b/README.md
@@ -118,6 +118,12 @@ You can also use watch mode to watch the entire directory for changes.
 qtsass ./static/scss -o ./static/css -w
 ```
 
+Set the Environment Variable QTSASS_DEBUG to 1 or pass the --debug flag to enable logging.
+
+```bash
+qtsass ./static/scss -o ./static/css --debug
+```
+
 ## API methods
 
 ### `compile(string, **kwargs)`
@@ -151,7 +157,7 @@ Examples:
 
 ```bash
 >>> import qtsass
->>> qtsass.compile_filename('dummy.scss', 'dummy.css') 
+>>> qtsass.compile_filename('dummy.scss', 'dummy.css')
 ```
 
 Arguments:
@@ -167,7 +173,7 @@ Examples:
 
 ```bash
 >>> import qtsass
->>> qtsass.compile_filename('dummy.scss', 'dummy.css') 
+>>> qtsass.compile_filename('dummy.scss', 'dummy.css')
 ```
 
 Arguments:
@@ -189,7 +195,28 @@ Arguments:
 - output_dir: Directory to write compiled Qt compliant CSS files to.
 - kwargs: Keyword arguments to pass to sass.compile
 
-### watch(source, destination, compiler=None, recursive=True):
+### `enable_logging(level=None, handler=None)`:
+Enable logging for qtsass.
+
+Sets the qtsass logger's level to:
+    1. the provided logging level
+    2. logging.DEBUG if the QTSASS_DEBUG envvar is a True value
+    3. logging.WARNING
+
+```bash
+>>> import logging
+>>> import qtsass
+>>> handler = logging.StreamHandler()
+>>> formatter = logging.Formatter('%(level)-8s: %(name)s> %(message)s')
+>>> handler.setFormatter(formatter)
+>>> qtsass.enable_logging(level=logging.DEBUG, handler=handler)
+```
+
+Arguments:
+- level: Optional logging level
+- handler: Optional handler to add
+
+### `watch(source, destination, compiler=None, Watcher=None)`:
 Watches a source file or directory, compiling QtSass files when modified.
 
 The compiler function defaults to compile_filename when source is a file
@@ -199,11 +226,10 @@ Arguments:
 - source: Path to source QtSass file or directory.
 - destination: Path to output css file or directory.
 - compiler: Compile function (optional)
-- recursive: If True, watch subdirectories (default: True).
-
-Returns: 
-- watchdog.Observer
+- Watcher: Defaults to qtsass.watchers.Watcher (optional)
 
+Returns:
+- qtsass.watchers.Watcher instance
 
 ## Contributing
 
diff --git a/qtsass/__init__.py b/qtsass/__init__.py
index adc34db..d8a0ee0 100644
--- a/qtsass/__init__.py
+++ b/qtsass/__init__.py
@@ -21,12 +21,25 @@ those variations.
 
 from __future__ import absolute_import
 
+# Standard library imports
+import logging
+
 # Local imports
-from qtsass.api import compile, compile_dirname, compile_filename, watch
+from qtsass.api import (
+    compile,
+    compile_dirname,
+    compile_filename,
+    enable_logging,
+    watch,
+)
 
 
 # yapf: enable
 
+# Setup Logging
+logging.getLogger(__name__).addHandler(logging.NullHandler())
+enable_logging()
+
 # Constants
 VERSION_INFO = (0, 1, 2, 'dev0')
 __version__ = '.'.join(map(str, VERSION_INFO))
diff --git a/qtsass/api.py b/qtsass/api.py
index 8391a1d..7ea4138 100644
--- a/qtsass/api.py
+++ b/qtsass/api.py
@@ -33,7 +33,6 @@ DEFAULT_CUSTOM_FUNCTIONS = {'qlineargradient': qlineargradient, 'rgba': rgba}
 DEFAULT_SOURCE_COMMENTS = False
 
 # Logger setup
-logging.basicConfig(level=logging.DEBUG)
 _log = logging.getLogger(__name__)
 
 
@@ -87,6 +86,14 @@ def compile(string, **kwargs):
         _log.error('Failed to conform source code')
         raise
 
+    if _log.isEnabledFor(logging.DEBUG):
+        from pprint import pformat
+        log_kwargs = dict(kwargs)
+        log_kwargs['string'] = 'Conformed SCSS<...>'
+        _log.debug('Calling sass.compile with:')
+        _log.debug(pformat(log_kwargs))
+        _log.debug('Conformed scss:\n{}'.format(kwargs['string']))
+
     # Compile QtSass source code
     try:
         return qt_conform(sass.compile(**kwargs))
@@ -114,7 +121,7 @@ def compile_filename(input_file, output_file, **kwargs):
     with open(input_file, 'r') as f:
         string = f.read()
 
-    _log.debug('Compiling {}...'.format(os.path.normpath(input_file)))
+    _log.info('Compiling {}...'.format(os.path.normpath(input_file)))
     css = compile(string, **kwargs)
 
     output_root = os.path.abspath(os.path.dirname(output_file))
@@ -124,6 +131,7 @@ def compile_filename(input_file, output_file, **kwargs):
     with open(output_file, 'w') as css_file:
         css_file.write(css)
         _log.info('Created CSS file {}'.format(os.path.normpath(output_file)))
+
     return css
 
 
@@ -161,6 +169,39 @@ def compile_dirname(input_dir, output_dir, **kwargs):
             compile_filename(scss_path, css_path, **fkwargs)
 
 
+def enable_logging(level=None, handler=None):
+    """Enable logging for qtsass.
+
+    Sets the qtsass logger's level to:
+        1. the provided logging level
+        2. logging.DEBUG if the QTSASS_DEBUG envvar is a True value
+        3. logging.WARNING
+
+    .. code-block:: python
+        >>> import logging
+        >>> import qtsass
+        >>> handler = logging.StreamHandler()
+        >>> formatter = logging.Formatter('%(level)-8s: %(name)s> %(message)s')
+        >>> handler.setFormatter(formatter)
+        >>> qtsass.enable_logging(level=logging.DEBUG, handler=handler)
+
+    :param level: Optional logging level
+    :param handler: Optional handler to add
+    """
+    if level is None:
+        debug = os.environ.get('QTSASS_DEBUG', False)
+        if debug in ('1', 'true', 'True', 'TRUE', 'on', 'On', 'ON'):
+            level = logging.DEBUG
+        else:
+            level = logging.WARNING
+
+    logger = logging.getLogger('qtsass')
+    logger.setLevel(level)
+    if handler:
+        logger.addHandler(handler)
+    _log.debug('logging level set to {}.'.format(level))
+
+
 def watch(source, destination, compiler=None, Watcher=None):
     """
     Watches a source file or directory, compiling QtSass files when modified.
diff --git a/qtsass/cli.py b/qtsass/cli.py
index 3305795..51ef727 100644
--- a/qtsass/cli.py
+++ b/qtsass/cli.py
@@ -21,12 +21,17 @@ import sys
 import time
 
 # Local imports
-from qtsass.api import compile, compile_dirname, compile_filename, watch
+from qtsass.api import (
+    compile,
+    compile_dirname,
+    compile_filename,
+    enable_logging,
+    watch,
+)
 
 
 # yapf: enable
 
-logging.basicConfig(level=logging.DEBUG)
 _log = logging.getLogger(__name__)
 
 
@@ -53,12 +58,35 @@ def create_parser():
         action='store_true',
         help='If set, recompile when the source file changes.',
     )
+    parser.add_argument(
+        '-d',
+        '--debug',
+        action='store_true',
+        help='Set the logging level to DEBUG.',
+    )
     return parser
 
 
 def main():
     """CLI entry point."""
     args = create_parser().parse_args()
+
+    # Setup CLI logging
+    debug = os.environ.get('QTSASS_DEBUG', args.debug)
+    if debug in ('1', 'true', 'True', 'TRUE', 'on', 'On', 'ON', True):
+        level = logging.DEBUG
+    else:
+        level = logging.INFO
+    enable_logging(level)
+
+    # Add a StreamHandler
+    handler = logging.StreamHandler()
+    if level == logging.DEBUG:
+        fmt = '%(levelname)-8s: %(name)s> %(message)s'
+        handler.setFormatter(logging.Formatter(fmt))
+    logging.root.addHandler(handler)
+    logging.root.setLevel(level)
+
     file_mode = os.path.isfile(args.input)
     dir_mode = os.path.isdir(args.input)
 
@@ -74,6 +102,7 @@ def main():
         sys.exit(0)
 
     elif file_mode:
+        _log.debug('compile_filename({}, {})'.format(args.input, args.output))
         compile_filename(args.input, args.output)
 
     elif dir_mode and not args.output:
@@ -81,6 +110,7 @@ def main():
         sys.exit(1)
 
     elif dir_mode:
+        _log.debug('compile_dirname({}, {})'.format(args.input, args.output))
         compile_dirname(args.input, args.output)
 
     else:
diff --git a/setup.cfg b/setup.cfg
index 727c878..b222ba0 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -38,6 +38,7 @@ blank_line_before_nested_class_or_def = false
 from_first = true
 import_heading_stdlib = Standard library imports
 import_heading_firstparty = Local imports
+import_heading_localfolder = Local imports
 import_heading_thirdparty = Third party imports
 indent = '    '
 known_first_party = qtsass
@@ -47,3 +48,5 @@ line_length = 79
 sections = FUTURE,STDLIB,THIRDPARTY,FIRSTPARTY,LOCALFOLDER
 lines_after_imports = 2
 skip = venv
+multi_line_output = 3
+include_trailing_comma = true


Test patch:
diff --git a/tests/test_api.py b/tests/test_api.py
index 0690df6..14fb31f 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -12,6 +12,7 @@ from __future__ import absolute_import
 
 # Standard library imports
 from os.path import exists
+import logging
 
 # Third party imports
 import pytest
@@ -20,6 +21,7 @@ import sass
 # Local imports
 import qtsass
 
+# Local imports
 from . import EXAMPLES_DIR, PROJECT_DIR, example
 
 
@@ -56,6 +58,14 @@ QWidget {
 """
 
 
+def setup_module():
+    qtsass.enable_logging(level=logging.DEBUG)
+
+
+def teardown_module():
+    qtsass.enable_logging(level=logging.WARNING)
+
+
 def test_compile_strings():
     """compile various strings."""
 
diff --git a/tests/test_cli.py b/tests/test_cli.py
index 39f31a4..6dca14c 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -16,7 +16,7 @@ from os.path import basename, exists
 from subprocess import PIPE, Popen
 import time
 
-#Local imports
+# Local imports
 from . import PROJECT_DIR, await_condition, example, touch
 
 
diff --git a/tests/test_watchers.py b/tests/test_watchers.py
index 185758d..a28cef0 100644
--- a/tests/test_watchers.py
+++ b/tests/test_watchers.py
@@ -25,6 +25,7 @@ from qtsass import compile_filename
 from qtsass.watchers import PollingWatcher, QtWatcher
 from qtsass.watchers.api import retry
 
+# Local imports
 from . import EXAMPLES_DIR, await_condition, example, touch