Merge pull request #14 from seapagan/fix-extra-spaces

Fix extra spaces and CR in output

README.md

@@ -23,22 +23,31 @@ Lice generates license files. No more hunting down licenses from other projects.
 > quite large, and no-one is merging PR's on the original project. Otherwise,
 > the Git history is identical to the original.
 
-This version fixes the compatibility issue and updates the tooling :
+This version fixes the compatibility issue with Python 3.12, and adds some new
+features:
 
 - It now uses [Poetry](https://python-poetry.org/) for dependency management
-- Can read from a config file for default values [`dev version only`]
-- Converted from 'argparse' to 'Typer' for CLI handling [`dev version only`]
+- Can read from a config file for default values
+- Converted from 'argparse' to 'Typer' for CLI handling
+- Fixes the issue where extra spaces and newlines were added to the generated
+  license text. This was considered a bug by at least several users, so it was
+  fixed in version `0.10.0`. However, if you want to generate a license with the
+  old style, you can use the `--legacy` option or set the `legacy` key in the
+  configuration file to `true`
 - The code has been modernized and cleaned up, all type-hinting has been
 added
 - It passes strict linting with the latest 'Ruff' and 'mypy'
-- Added a documentation site.
 - GitHub actions set up for linting, `Dependabot` and `Dependency Review`
 
 In addition, future plans can be seen in the [TODO.md](TODO.md) file.
 
 > [!IMPORTANT]
 > This appllication is now only compatible with Python 3.9 and above. If you
 > wish to use an older version, use the original 'lice' package.
+>
+> However, I'ts the **development** dependencies that are causing the
+> incompatibility, so I'll look at reducing the **Production** version in future
+> releases while still requiring Python 3.9 or above for development.
 
 ## Installation
 

docs/configuration.md

@@ -15,12 +15,18 @@ The TOML file should look like this:
 [lice]
 default_license = "mit"
 organization = "Your Organization"
+legacy = true
 ```
 
-Currently there are only two options that can be set:
+Currently there are only three options that can be set:
 
 - `default_license` - This is the default license that will be used if no
-  license is specified on the command line.
+  license is specified on the command line. If this option is not set, it will
+  default to `bsd3`.
 - `organization` - This is the organization name that will be used in the
   license by default. If this is set, it will not try to get the organization
   name from `git config` or the `$USER` environment variable.
+- `legacy` - This is a boolean value that will set the default style license
+  generation to the old style with extra spaces and newlines. If this option is
+  not set, it will default to `false`. See the [--legacy
+  option](usage.md#-legacy-option) for more information.

docs/index.md

@@ -11,12 +11,18 @@ Lice generates license files. No more hunting down licenses from other projects.
     quite large, and no-one is merging PR's on the original project. Otherwise,
     the Git history is preserved from the original.
 
-This version fixes the compatibility issue and updates the tooling :
+This version fixes the compatibility issue with Python 3.12, and adds some new
+features:
 
 - It now uses [Poetry](https://python-poetry.org/){:target="_blank"} for
   dependency management
 - Can read from a config file for default values
 - Converted from 'argparse' to 'Typer' for CLI handling
+- Fixes the issue where extra spaces and newlines were added to the generated
+  license text. This was considered a bug by at least several users, so it was
+  fixed in version `0.10.0`. However, if you want to generate a license with the
+  old style, you can use the `--legacy` option or set the `legacy` key in the
+  configuration file to `true`
 - The code has been modernized and cleaned up, all type-hinting has been
 added
 - It passes strict linting with the latest 'Ruff' and 'mypy'
@@ -28,3 +34,7 @@ page.
 !!! warning "Python Compatibility"
     This appllication is now only compatible with Python 3.9 and above. If you
     wish to use an older version, use the original 'lice' package.
+
+    However, I'ts the **development** dependencies that are causing the
+    incompatibility, so I'll look at reducing the **Production** version in
+    future releases while still requiring Python 3.9 or above for development.

docs/usage.md

@@ -125,6 +125,25 @@ lice mit -f "LICENSE.txt"
     If you specify a language with the `-l` option, the extension will be
     automatically added to the file name so you don't need to include it.
 
+### `--legacy` option
+
+In the original `lice`, the licenses were generated with a leading space on each
+line and extra newlines at start and end. This was considered a bug by at least
+several users, so it was fixed in version 0.9.1. However, if you want to
+generate a license with the old style, you can use the `--legacy` option.
+
+```console
+lice mit --legacy
+```
+
+If you want to use the old style by default, you can set the `legacy` key in the
+configuration file to `true`.
+
+```toml
+[lice]
+legacy = true
+```
+
 ### `--vars` option
 
 This will list the variables that can be used in the specified license.

lice2/config.py

@@ -12,6 +12,7 @@ class Settings(TOMLSettings):
 
     default_license: str = "bsd3"
     organization: str = ""
+    legacy: bool = False
 
 
 def check_default_license() -> str:

lice2/core.py

@@ -11,7 +11,7 @@
 import typer
 from rich.markup import escape  # noqa: TCH002
 
-from lice2.config import check_default_license
+from lice2.config import check_default_license, settings
 from lice2.constants import LANGS, LICENSES  # noqa: TCH001
 from lice2.helpers import (
     format_license,
@@ -130,6 +130,13 @@ def main(  # noqa: PLR0913
             help="List available source code formatting languages",
         ),
     ] = False,
+    legacy: Annotated[
+        bool,
+        typer.Option(
+            "--legacy",
+            help="Use legacy method to generate license",
+        ),
+    ] = False,
 ) -> None:
     """Generate a license file.
 
@@ -146,6 +153,7 @@ def main(  # noqa: PLR0913
         "year": year,
         "language": language,
         "ofile": ofile,
+        "legacy": legacy or settings.legacy,
         "list_vars": show_vars,
         "list_licenses": show_licenses,
         "list_languages": show_languages,
@@ -184,17 +192,19 @@ def main(  # noqa: PLR0913
         ext = get_suffix(args.ofile)
         if ext:
             output = args.ofile
-            out = format_license(content, ext)  # format license by file suffix
+            out = format_license(
+                content, ext, legacy=args.legacy
+            )  # format license by file suffix
         else:
             output = f"{args.ofile}.{lang}" if lang else args.ofile
-            out = format_license(content, lang)
+            out = format_license(content, lang, legacy=args.legacy)
 
         out.seek(0)
         with Path(output).open(mode="w") as f:
             f.write(out.getvalue())
         f.close()
     else:
-        out = format_license(content, lang)
+        out = format_license(content, lang, legacy=args.legacy)
         out.seek(0)
         sys.stdout.write(out.getvalue())
     out.close()  # free content memory (paranoic memory stuff)

lice2/helpers.py

@@ -141,36 +141,66 @@ def generate_license(template: StringIO, context: dict[str, str]) -> StringIO:
 
     We extract variables from the template and replace them with the
     corresponding values in the given context.
+
+    This could be done with a template engine like 'Jinja2, but we're keeping it
+    simple.
     """
     out = StringIO()
-    content = template.getvalue()
-    for key in extract_vars(template):
-        if key not in context:
-            message = f"{key} is missing from the template context"
-            raise ValueError(message)
-        content = content.replace(f"{{{{ {key} }}}}", context[key])
-    template.close()  # free template memory (when is garbage collected?)
-    out.write(content)
+    with closing(template):
+        content = template.getvalue()
+        for key in extract_vars(template):
+            if key not in context:
+                message = f"{key} is missing from the template context"
+                raise ValueError(message)
+            content = content.replace(f"{{{{ {key} }}}}", context[key])
+        out.write(content)
     return out
 
 
-def format_license(template: StringIO, lang: str) -> StringIO:
+def get_comments(lang: str, *, legacy: bool) -> tuple[str, str, str]:
+    """Adjust the comment strings for the given language.
+
+    The way it was done previously, extra whitespace was added to the start of
+    the comment lines if the comment was a block comment. This tries to fix
+    that.
+    """
+    prefix, comment, postfix = LANG_CMT[LANGS[lang]]
+    if legacy:
+        return (
+            f"{prefix}\n",
+            f"{comment} ",
+            f"{postfix}\n",
+        )
+
+    if comment:
+        comment = f"{comment} "
+    prefix = f"{prefix}\n" if prefix else ""
+    postfix = f"{postfix}\n" if postfix else ""
+    return prefix, comment, postfix
+
+
+def format_license(
+    template: StringIO, lang: str, *, legacy: bool = False
+) -> StringIO:
     """Format the StringIO template object for specified lang string.
 
     Return StringIO object formatted
     """
     if not lang:
         lang = "txt"
 
+    prefix, comment, postfix = get_comments(lang, legacy=legacy)
+
     out = StringIO()
 
     with closing(template):
         template.seek(0)  # from the start of the buffer
-        out.write(LANG_CMT[LANGS[lang]][0] + "\n")
+        out.write(prefix)
         for line in template:
-            out.write(LANG_CMT[LANGS[lang]][1] + " ")
+            # ensure no extra whitespace is added for blank lines
+            out.write(comment if line.strip() else comment.strip())
             out.write(line)
-        out.write(LANG_CMT[LANGS[lang]][2] + "\n")
+        out.write(postfix)
 
     return out
 
@@ -232,11 +262,11 @@ def generate_header(args: SimpleNamespace, lang: str) -> None:
             )
             raise typer.Exit(1) from None
 
-    content = generate_license(template, get_context(args))
-    out = format_license(content, lang)
-    out.seek(0)
-    sys.stdout.write(out.getvalue())
-    out.close()  # free content memory (paranoic memory stuff)
+    with closing(template):
+        content = generate_license(template, get_context(args))
+        out = format_license(content, lang, legacy=args.legacy)
+        out.seek(0)
+        sys.stdout.write(out.getvalue())
     raise typer.Exit(0)
 
 

lice2/tests/conftest.py

@@ -11,9 +11,7 @@
 if TYPE_CHECKING:
     from pyfakefs.fake_filesystem import FakeFilesystem
 
-TEMPLATE_FILE = """
-This is a template file.
-
+TEMPLATE_FILE = """This is a template file.
 {{ organization }} is the organization.
 {{ project }} is the project.
 {{ year }} is the year.
@@ -51,6 +49,7 @@ def args() -> SimpleNamespace:
         "year": "2024",
         "language": None,
         "ofile": None,
+        "legacy": False,
         "list_vars": False,
         "list_licenses": False,
         "list_languages": False,

lice2/tests/test_lice.py

@@ -363,20 +363,30 @@ def test_generate_license_missing_context(mocker: MockerFixture) -> None:
     mock_out_instance.write.assert_not_called()
 
 
-def test_format_license_no_lang() -> None:
-    """Test the 'format_license' function."""
+def test_format_license_no_lang_legacy(fake_config) -> None:
+    """Test the 'format_license' function with no lang and legacy=True."""
     content = StringIO(TEMPLATE_FILE)
-    result = format_license(content, "")
+    result = format_license(content, "", legacy=True)
 
     # Adjust the TEMPLATE_FILE to match the expected output with a leading space
-    # on each line. This extra space is added by the 'format_license' function
-    # but may be removed in future versions as it's not really correct. This
-    # test will need to be updated if that happens.
-    adjusted_template = "\n".join(
-        " " + line for line in TEMPLATE_FILE.splitlines()
+    # on each line and leading/post <CR>. This extra space is added when the
+    # '--legacy' flag is used, to maintain compatibility with the original lice
+    # if required.
+    adjusted_template = (
+        "\n"
+        + ("\n".join(" " + line for line in TEMPLATE_FILE.splitlines()) + "\n")
+        + "\n"
     )
 
-    assert result.getvalue().strip() == adjusted_template.strip()
+    assert result.getvalue() == adjusted_template
+
+
+def test_format_license_no_lang() -> None:
+    """Test the 'format_license' function."""
+    content = StringIO(TEMPLATE_FILE)
+    result = format_license(content, "")
+
+    assert result.getvalue() == TEMPLATE_FILE
 
 
 def test_load_file_template_path_not_found() -> None: