From 6ad8a043cae3d4e0db906788cb60a4c8a3fd7d91 Mon Sep 17 00:00:00 2001 From: Adrian Date: Tue, 30 Jun 2026 12:05:42 +0200 Subject: [PATCH] Fix genspider --editor (#7683) --- scrapy/commands/edit.py | 23 +++++++++++++-- scrapy/commands/genspider.py | 12 +++++--- tests/test_command_genspider.py | 50 +++++++++++++++++++++++++++++++++ tests/test_commands.py | 28 ++++++++++++++++++ 4 files changed, 106 insertions(+), 7 deletions(-) diff --git a/scrapy/commands/edit.py b/scrapy/commands/edit.py index cd7c57f28..fa75cb09c 100644 --- a/scrapy/commands/edit.py +++ b/scrapy/commands/edit.py @@ -1,12 +1,29 @@ -import argparse +from __future__ import annotations + import os +import shlex +import subprocess import sys -from typing import Any, ClassVar +from pathlib import Path +from typing import TYPE_CHECKING, Any, ClassVar from scrapy.commands import ScrapyCommand from scrapy.exceptions import UsageError from scrapy.spiderloader import get_spider_loader +if TYPE_CHECKING: + import argparse + + +def _edit_file(editor: str, file_path: str | os.PathLike[str]) -> int: + """Open ``file_path`` with ``editor`` and return the editor exit code. + + ``editor`` may include arguments (e.g. ``"code -w"``); it is split with + :func:`shlex.split` and the file is passed as a separate argument, so no + shell is involved. + """ + return subprocess.call([*shlex.split(editor), os.fspath(file_path)]) # noqa: S603 + class Command(ScrapyCommand): requires_project = True @@ -45,4 +62,4 @@ class Command(ScrapyCommand): sfile = sys.modules[spidercls.__module__].__file__ assert sfile sfile = sfile.replace(".pyc", ".py") - self.exitcode = os.system(f'{editor} "{sfile}"') # noqa: S605 + self.exitcode = _edit_file(editor, Path(sfile)) diff --git a/scrapy/commands/genspider.py b/scrapy/commands/genspider.py index cc8624fa1..4277232c3 100644 --- a/scrapy/commands/genspider.py +++ b/scrapy/commands/genspider.py @@ -1,6 +1,5 @@ from __future__ import annotations -import os import shutil import string from importlib import import_module @@ -10,12 +9,14 @@ from urllib.parse import urlparse import scrapy from scrapy.commands import ScrapyCommand +from scrapy.commands.edit import _edit_file from scrapy.exceptions import UsageError from scrapy.spiderloader import get_spider_loader from scrapy.utils.template import render_templatefile, string_camelcase if TYPE_CHECKING: import argparse + import os def sanitize_module_name(module_name: str) -> str: @@ -118,9 +119,11 @@ class Command(ScrapyCommand): template_file = self._find_template(opts.template) if template_file: - self._genspider(module, name, url, opts.template, template_file) + spider_file = self._genspider( + module, name, url, opts.template, template_file + ) if opts.edit: - self.exitcode = os.system(f'scrapy edit "{name}"') # noqa: S605 + self.exitcode = _edit_file(self.settings["EDITOR"], spider_file) def _generate_template_variables( self, @@ -148,7 +151,7 @@ class Command(ScrapyCommand): url: str, template_name: str, template_file: str | os.PathLike[str], - ) -> None: + ) -> Path: """Generate the spider module, based on the given template""" assert self.settings is not None tvars = self._generate_template_variables(module, name, url, template_name) @@ -168,6 +171,7 @@ class Command(ScrapyCommand): ) if spiders_module: print(f"in module:\n {spiders_module.__name__}.{module}") + return Path(spider_file) def _find_template(self, template: str) -> Path | None: template_file = Path(self.templates_dir, f"{template}.tmpl") diff --git a/tests/test_command_genspider.py b/tests/test_command_genspider.py index 67e3eb50a..94db14c30 100644 --- a/tests/test_command_genspider.py +++ b/tests/test_command_genspider.py @@ -1,6 +1,7 @@ from __future__ import annotations import re +import sys from pathlib import Path import pytest @@ -9,6 +10,13 @@ from tests.test_commands import TestProjectBase from tests.utils.cmdline import call, proc +def write_recording_editor(editor: Path) -> None: + """Create an executable editor script that writes the path it is asked to + open (its last argument) into the file given as its first argument.""" + editor.write_text('#!/bin/sh\nprintf "%s" "$2" > "$1"\n', encoding="utf-8") + editor.chmod(0o755) + + def find_in_file(filename: Path, regex: str) -> re.Match[str] | None: """Find first pattern occurrence in file""" pattern = re.compile(regex) @@ -63,6 +71,28 @@ class TestGenspiderCommand(TestProjectBase): assert call("genspider", "--dump=basic", cwd=proj_path) == 0 assert call("genspider", "-d", "basic", cwd=proj_path) == 0 + @pytest.mark.skipif( + sys.platform == "win32", reason="requires a POSIX shell editor script" + ) + def test_edit(self, proj_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + spider = proj_path / self.project_name / "spiders" / "example2.py" + edited = proj_path / "edited.txt" + editor = proj_path / "fake-editor.sh" + write_recording_editor(editor) + # The extra argument exercises shlex-splitting of the EDITOR value. + monkeypatch.setenv("EDITOR", f"{editor} {edited}") + + returncode, _, err = proc( + "genspider", "--edit", "example2", "example2.com", cwd=proj_path + ) + + assert returncode == 0, err + assert "ModuleNotFoundError" not in err + assert spider.exists() + assert (proj_path / edited.read_text(encoding="utf-8")).resolve() == ( + spider.resolve() + ) + def test_same_name_as_project(self, proj_path: Path) -> None: assert call("genspider", self.project_name, cwd=proj_path) == 2 assert not ( @@ -168,6 +198,26 @@ class TestGenspiderStandaloneCommand: call("genspider", "example", "example.com", cwd=tmp_path) assert Path(tmp_path, "example.py").exists() + @pytest.mark.skipif( + sys.platform == "win32", reason="requires a POSIX shell editor script" + ) + def test_edit(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + spider = tmp_path / "example.py" + edited = tmp_path / "edited.txt" + editor = tmp_path / "fake-editor.sh" + write_recording_editor(editor) + monkeypatch.setenv("EDITOR", f"{editor} {edited}") + + returncode, _, err = proc( + "genspider", "--edit", "example", "example.com", cwd=tmp_path + ) + + assert returncode == 0, err + assert spider.exists() + assert (tmp_path / edited.read_text(encoding="utf-8")).resolve() == ( + spider.resolve() + ) + @pytest.mark.parametrize("force", [True, False]) def test_same_name_as_existing_file(self, force: bool, tmp_path: Path) -> None: file_name = "example" diff --git a/tests/test_commands.py b/tests/test_commands.py index d7b7a9ff2..9f81e8602 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -2,6 +2,7 @@ from __future__ import annotations import argparse import json +import sys from io import StringIO from shutil import copytree from typing import TYPE_CHECKING @@ -420,6 +421,33 @@ class TestViewCommand: assert "URL using the Scrapy downloader and show its" in command.long_desc() +class TestEditCommand(TestProjectBase): + @pytest.mark.skipif( + sys.platform == "win32", reason="requires a POSIX shell editor script" + ) + def test_edit(self, proj_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + spider = proj_path / self.project_name / "spiders" / "example.py" + edited = proj_path / "edited.txt" + editor = proj_path / "fake-editor.sh" + # Records the file it is asked to open ($2) into the file given as $1. + editor.write_text('#!/bin/sh\nprintf "%s" "$2" > "$1"\n', encoding="utf-8") + editor.chmod(0o755) + monkeypatch.setenv("EDITOR", f"{editor} {edited}") + + assert call("genspider", "example", "example.com", cwd=proj_path) == 0 + returncode, _, err = proc("edit", "example", cwd=proj_path) + + assert returncode == 0, err + assert (proj_path / edited.read_text(encoding="utf-8")).resolve() == ( + spider.resolve() + ) + + def test_edit_spider_not_found(self, proj_path: Path) -> None: + returncode, _, err = proc("edit", "nonexistent", cwd=proj_path) + assert returncode == 1 + assert "Spider not found: nonexistent" in err + + class TestHelpMessage(TestProjectBase): @pytest.mark.parametrize( "command",