[warosu] Site change

[NecRaul]

Very similar to #7676.

I'll reach out to the admin to see what's causing these issues. I'm not 100% sure, but it might have to do with this line because what I’m seeing in the browser and what I’m seeing in the debugger don’t match one-to-one:

gallery-dl/gallery_dl/extractor/warosu.py

Line 32 in a4fb9e6

page = self.request(url).text

In the meantime, it might be a good idea to rethink this extractor and maybe introduce something like bs4 and lxml for easier parsing of html. I don't know how open you'll be to that idea though. Maybe on v2.0?

[mikf]

maybe introduce something like bs4 and lxml for easier parsing of html

No. Maybe regular expressions instead of searching for static strings, given that 4fc719b is now a thing, but no extra dependencies for things that can be done without them.

[NecRaul]

Just as I was gonna comment again, I noticed you merged already.

I thought about what you said and although I don't agree with the sentiment and I think using the right tool for the right job would make everyone's life easier, but alas you do you. In the meantime, I tried working with 4fc719b and this is what I got:

diff --git i/gallery_dl/extractor/warosu.py w/gallery_dl/extractor/warosu.py
index e78fd949..41f44d60 100644
--- i/gallery_dl/extractor/warosu.py
+++ w/gallery_dl/extractor/warosu.py
@@ -49,13 +49,13 @@ class WarosuThreadExtractor(Extractor):
                 yield Message.Url, post["image"], post
 
     def metadata(self, page):
-        boardname = text.extr(page, "<title>", "</title>")
-        title = text.unescape(text.extr(page, "class=filetitle>", "<"))
+        boardname_pattern = r" - ([^-]+) - "
+        title_pattern = r'<meta[^>]+content="([^"]+)"[^>]*property=og:title'
         return {
             "board"     : self.board,
-            "board_name": boardname.split(" - ")[1],
+            "board_name": self._rex(boardname_pattern, page),
             "thread"    : self.thread,
-            "title"     : title,
+            "title"     : self._rex(title_pattern, page),
         }
 
     def posts(self, page):
@@ -67,40 +67,59 @@ class WarosuThreadExtractor(Extractor):
     def parse(self, post):
         """Build post object by extracting data from an HTML post"""
         data = self._extract_post(post)
-        if '<span class="fileinfo' in post and \
-                self._extract_image(post, data):
-            part = data["image"].rpartition("/")[2]
-            data["tim"], _, data["extension"] = part.partition(".")
-            data["ext"] = "." + data["extension"]
+
+        fileinfo_pattern = (
+            r'File:\s*([^,]+),\s*'        # fsize
+            r'(\d+)\s*x\s*(\d+),\s*'      # w x h
+            r'([^\s<>"]+\.[a-zA-Z0-9]+)'  # filename.ext
+        )
+        if not text.re(fileinfo_pattern).search(post):
+            return data
+
+        if not self._extract_image(post, data, fileinfo_pattern):
+            return data
+
+        part = data["image"].rpartition("/")[2]
+        data["tim"], _, data["extension"] = part.partition(".")
+        data["ext"] = "." + data["extension"]
         return data
 
     def _extract_post(self, post):
-        extr = text.extract_from(post)
+        no = self._rex(r'id=p(\d+)', post),
+        name = self._rex(r'class=postername>([^<]+)', post).strip(),
+        time = self._rex(r'class=posttime\s+title=["\']?(\d+)["\']?', post),
+        com = text.unescape(text.remove_html(
+            self._rex(r'<blockquote>(.*?)</blockquote>', post).strip())),
         return {
-            "no"  : extr("id=p", ">"),
-            "name": extr("class=postername>", "<").strip(),
-            "time": extr("class=posttime title=", "000>"),
-            "com" : text.unescape(text.remove_html(extr(
-                "<blockquote>", "</blockquote>").strip())),
+            "no"  : no,
+            "name": name,
+            "time": time,
+            "com" : com,
         }
 
-    def _extract_image(self, post, data):
-        extr = text.extract_from(post)
-        extr('<span class="fileinfo">', "")
-        data["fsize"] = extr("File: ", ", ")
-        data["w"] = extr("", "x")
-        data["h"] = extr("", ", ")
-        data["filename"] = text.unquote(extr(
-            "", "<").rstrip().rpartition(".")[0])
-        extr("<br>", "")
-
-        url = extr("<a href=", ">")
+    def _extract_image(self, post, data, fileinfo_pattern):
+        data["fsize"] = self._rex(fileinfo_pattern, post, 1).strip()
+        data["w"] = self._rex(fileinfo_pattern, post, 2)
+        data["h"] = self._rex(fileinfo_pattern, post, 3)
+        filename_raw = self._rex(fileinfo_pattern, post, 4).strip()
+        data["filename"] = text.unquote(filename_raw.rpartition('.')[0])
+
+        url_pattern = (
+            r'<a\s+href=([\'"]?)'               # <a href= with optional quote
+            r'([^\'" >]+?\.(?:png|jpe?g|gif))'  # URL ending with image ext
+            r'\1>'                              # closing quote and >
+            r'\s*<img\b'                        # followed by <img (child)
+        )
+        url = self._rex(url_pattern, post, 2)
         if url:
             if url[0] == "/":
                 data["image"] = self.root + url
-            elif "warosu." not in url:
-                return False
             else:
                 data["image"] = url
             return True
         return False
+
+    def _rex(self, pattern, text_, num=1):
+        """Helper: extract "num" group from regex search with caching."""
+        match = text.re(pattern).search(text_)
+        return match.group(num) if match else ""

[NecRaul]

The code above returns the same results as its text.extr counterpart, but due to regex matching, it's more robust.

I can submit a PR to change from text.extr to using text.re in warosu.py extractor, which is more resilient.

I implemented the _rex helper method because text.re(pattern).search(text_).group(num) was being called multiple times, and repeating that every time is rather cumbersome.

Using:

value = self._rex(pattern, text_, num)

is preferable to:

re_pattern = text.re(pattern)
match = re_pattern.search(text_)
value = match.group(num)

I believe this helper should be implemented in util.py or text.py as a function, rather than redefined in each extractor it's used in.

value = text.rex(pattern, text_, num)

I also removed code for checking hostname from #7699 because with the url_pattern being used right now, it will always return the warosu image URL (data, and not thumb).

The only improvement I can think of is changing the parse method to also use text.re instead of text.extr but I couldn't figure out how to return an iterable set of posts.

[mikf]

would make everyone's life easier

How would extra dependencies make the life of a regular user easier? All it would do for them is make gallery-dl even slower and resource intensive than it already is.

Maybe it would make the data extraction code more durable, but the last time it broke for warosu was a year ago (0fcd603) and I'd rather not spend extra resources on fancy tools for a chance that things don't break as "often".

---

+        data["fsize"] = self._rex(fileinfo_pattern, post, 1).strip()
+        data["w"] = self._rex(fileinfo_pattern, post, 2)
+        data["h"] = self._rex(fileinfo_pattern, post, 3)
+        filename_raw = self._rex(fileinfo_pattern, post, 4).strip()
``

This does the same search four times, even though one would be all that is needed.

+        if not text.re(fileinfo_pattern).search(post):
+            return data

Never mind, five times...

I believe this helper should be implemented in util.py or text.py as a function, rather than redefined in each extractor it's used in.

util.re(…).search(txt) is used only twice throughout the entire code base.

[NecRaul]

How would extra dependencies make the life of a regular user easier? All it would do for them is make gallery-dl even slower and resource intensive than it already is.

Maybe it would make the data extraction code more durable, but the last time it broke for warosu was a year ago 0fcd603 and I'd rather not spend extra resources on fancy tools for a chance that things don't break as "often".

Fair point — I meant it’d make dev work easier, not necessarily help users. Tools like bs4 + lxml are just way more robust and readable than manually parsing or using regex — kinda like the difference between using querySelector and trying to walk the DOM manually in JavaScript.

This does the same search four times, even though one would be all that is needed.

Never mind, five times...

You're totally right. I don't know how that escaped me.

util.re(…).search(txt) is used only twice throughout the entire code base.

I should've been more clear in that statement. What I meant was: I believe this helper should be implemented ... if the move towards regex instead of static string checking is going to happen. Not just in this extractor but across the board.

I’ve rewritten the patch — if it looks acceptable to you, I can open a PR. If it still doesn’t seem right (or you’d prefer not to move towards util.re yet), that’s completely fine too.

diff --git i/gallery_dl/extractor/warosu.py w/gallery_dl/extractor/warosu.py
index e78fd949..c9798aba 100644
--- i/gallery_dl/extractor/warosu.py
+++ w/gallery_dl/extractor/warosu.py
@@ -9,7 +9,7 @@
 """Extractors for https://warosu.org/"""
 
 from .common import Extractor, Message
-from .. import text
+from .. import text, util
 
 
 class WarosuThreadExtractor(Extractor):
@@ -49,13 +49,13 @@ class WarosuThreadExtractor(Extractor):
                 yield Message.Url, post["image"], post
 
     def metadata(self, page):
-        boardname = text.extr(page, "<title>", "</title>")
-        title = text.unescape(text.extr(page, "class=filetitle>", "<"))
+        boardname_pattern = r" - ([^-]+) - "
+        title_pattern = r'<meta[^>]+content="([^"]+)"[^>]*property=og:title'
         return {
             "board"     : self.board,
-            "board_name": boardname.split(" - ")[1],
+            "board_name": self._rex(boardname_pattern, page),
             "thread"    : self.thread,
-            "title"     : title,
+            "title"     : self._rex(title_pattern, page),
         }
 
     def posts(self, page):
@@ -67,40 +67,62 @@ class WarosuThreadExtractor(Extractor):
     def parse(self, post):
         """Build post object by extracting data from an HTML post"""
         data = self._extract_post(post)
-        if '<span class="fileinfo' in post and \
-                self._extract_image(post, data):
-            part = data["image"].rpartition("/")[2]
-            data["tim"], _, data["extension"] = part.partition(".")
-            data["ext"] = "." + data["extension"]
+
+        if "fileinfo" not in post:
+            return data
+
+        if not self._extract_image(post, data):
+            return data
+
+        part = data["image"].rpartition("/")[2]
+        data["tim"], _, data["extension"] = part.partition(".")
+        data["ext"] = "." + data["extension"]
         return data
 
     def _extract_post(self, post):
-        extr = text.extract_from(post)
+        no = self._rex(r'id=p(\d+)', post),
+        name = self._rex(r'class=postername>([^<]+)', post).strip(),
+        time = self._rex(r'class=posttime\s+title=["\']?(\d+)["\']?', post),
+        com = text.unescape(text.remove_html(
+            self._rex(r'<blockquote>(.*?)</blockquote>', post).strip())),
         return {
-            "no"  : extr("id=p", ">"),
-            "name": extr("class=postername>", "<").strip(),
-            "time": extr("class=posttime title=", "000>"),
-            "com" : text.unescape(text.remove_html(extr(
-                "<blockquote>", "</blockquote>").strip())),
+            "no"  : no,
+            "name": name,
+            "time": time,
+            "com" : com,
         }
 
     def _extract_image(self, post, data):
-        extr = text.extract_from(post)
-        extr('<span class="fileinfo">', "")
-        data["fsize"] = extr("File: ", ", ")
-        data["w"] = extr("", "x")
-        data["h"] = extr("", ", ")
-        data["filename"] = text.unquote(extr(
-            "", "<").rstrip().rpartition(".")[0])
-        extr("<br>", "")
-
-        url = extr("<a href=", ">")
-        if url:
-            if url[0] == "/":
-                data["image"] = self.root + url
-            elif "warosu." not in url:
-                return False
-            else:
-                data["image"] = url
-            return True
-        return False
+        fileinfo_pattern = (
+            r'File:\s*([^,]+),\s*'        # fsize
+            r'(\d+)\s*x\s*(\d+),\s*'      # w x h
+            r'([^\s<>"]+\.[a-zA-Z0-9]+)'  # filename.ext
+        )
+        match = util.re(fileinfo_pattern).search(post)
+        if not match:
+            return False
+        data["fsize"] = match.group(1).strip()
+        data["w"] = match.group(2)
+        data["h"] = match.group(3)
+        filename_raw = match.group(4).strip()
+        data["filename"] = text.unquote(filename_raw.rpartition('.')[0])
+
+        url_pattern = (
+            r'<a\s+href=([\'"]?)'               # <a href= with optional quote
+            r'([^\'" >]+?\.(?:png|jpe?g|gif))'  # URL ending with image ext
+            r'\1>'                              # closing quote and >
+            r'\s*<img\b'                        # followed by <img (child)
+        )
+        url = self._rex(url_pattern, post, 2)
+        if url == "":
+            return False
+        elif url[0] == "/":
+            data["image"] = self.root + url
+        else:
+            data["image"] = url
+        return True
+
+    def _rex(self, pattern, text_, num=1):
+        """Helper: extract "num" group from regex search with caching."""
+        match = util.re(pattern).search(text_)
+        return match.group(num) if match else ""

[mikf]

I meant it’d make dev work easier, not necessarily help users

There are at least 1k users and only a few devs. I'd rather prioritize the 1k+.

I also prefer writing and using my own tools if possible instead of relying on other's and having to deal with their potential shortcomings etc.

Optional dependencies, when used only when a user decides to use a certain feature or option, are OK in my opinion, but hard requirements are something I'd like to avoid if possible. Similar to how youtube-dl and yt-dlp have zero required dependencies.

if the move towards regex instead of static string checking is going to happen

There are no plans to replace the current data extraction methods with regex.

I’ve rewritten the patch — if it looks acceptable to you, I can open a PR. If it still doesn’t seem right (or you’d prefer not to move towards util.re yet), that’s completely fine too.

The current code works as is, thanks to you and your changes. I think it would be better to not change anything until the next time it breaks.