Fix some issues to work on my ceiba #16

b04505009 · 2020-12-09T13:52:03Z

I fixed some issues in your project and removed some assertion based on my ceiba courses.

New features

Make sure the file path would be correct before creating file by library pathvalidate

Unexpected conditions on my ceiba

成績分佈 has 公佈全班 attribute, which is a link for whole class' grade
旁聽 courses is on table 2
I have time slot with space value (' ') for my 暑期實習
Students from NTNU or NTUST may not have all 12 rows in their pages
I have a course that it's 課程助教 is plain text

…ith '公佈全班' attribute, and comment out some assertions base on my course list

ryucc · 2021-04-05T00:55:31Z

ceiba_dl/vfs.py

@@ -641,144 +643,160 @@ def fetch(self):
        assert len(student_page.xpath('//table')) > 0

        student_rows = student_page.xpath('//div[@id="sect_cont"]/table/tr')
-        assert len(student_rows) == 12
+        # NTNU and NTUST students may have less rows
+        #assert len(student_rows) == 12


@ryucc Added

我其實有點好奇你遇到的是什麼狀況，不知道能不能貼個截圖（去除個資）當作參考，同時也讓之後看到這個 pull request 的人能更了解這個問題？我目前看過的 ntnu_* 和 ntust_* 帳號都和普通的臺大帳號一樣有 12 列，不知道是不是只有特定帳號有這個情形。

lantw44 · 2022-01-31T17:03:48Z

抱歉過了這麼久才回來看，有些關於 commit 的問題希望可以先解決：

你的 4 個 commit 的作者不一致，建議改成一樣的。如果不改的話，我會幫你改成可以對得到 GitHub 帳號的那個。

Author:     b04505009 <[email protected]>
Author:     b04505009 <[email protected]>
Author:     b04505009 <[email protected]>
Author:     b04505009 <[email protected]>

請不要修改檔案屬性，ceiba_dl 資料夾裡的檔案不該是可執行檔。

diff --git a/ceiba_dl/__init__.py b/ceiba_dl/__init__.py
old mode 100644
new mode 100755
diff --git a/ceiba_dl/config.py b/ceiba_dl/config.py
old mode 100644
new mode 100755
diff --git a/ceiba_dl/vfs.py b/ceiba_dl/vfs.py
old mode 100644
new mode 100755

除非你的 commit 會影響整個專案，不然請用「分類: 標題」的格式寫 commit 訊息。為了讓專案內的 commit 訊息格式一致，建議寫的時候可以參考一下現有的 commit。

lantw44 · 2022-01-31T18:31:44Z

ceiba_dl/__init__.py

@@ -10,6 +10,8 @@
 import pycurl
 import urllib.parse

+from pathvalidate import sanitize_filepath


請仿照其他檔案，把所有的 from ... import ... 排在一起，並照字母排序。

由於 pathvalidate 不在 Python 標準函式庫中，需要額外安裝才能使用，因此請一併修改 README.asciidoc、configure.ac、requirements.txt 讓使用者知道這件事。

lantw44 · 2022-01-31T18:38:17Z

ceiba_dl/__init__.py

@@ -283,7 +286,8 @@ def download_link(self, path, node, retry, dcb, ecb):

    def download_regular(self, path, node, retry, dcb, ecb):
        disk_path_object = pathlib.Path(path.lstrip('/'))
-
+        disk_path_object = pathlib.Path(sanitize_filepath(str(disk_path_object)))


如同我在 #15 (comment) 所說，我覺得這個可以移入 disk_path_object_open，只在檔名真的造成問題的時候才使用 sanitize_filepath。至於特殊字元的處理方式，我覺得可以比照現在 / 替換成 _ 的模式，看是要替換成 _ 還是其他相似或全形的符號，而不要直接刪除。

lantw44 · 2022-01-31T18:39:46Z

ceiba_dl/__init__.py

@@ -283,7 +286,8 @@ def download_link(self, path, node, retry, dcb, ecb):

    def download_regular(self, path, node, retry, dcb, ecb):
        disk_path_object = pathlib.Path(path.lstrip('/'))
-
+        disk_path_object = pathlib.Path(sanitize_filepath(str(disk_path_object)))
+


請刪除行尾空白。

lantw44 · 2022-01-31T18:52:07Z

ceiba_dl/config.py

@@ -90,6 +90,7 @@ class Config:
            'attr_course_grades_show': '成績公布',
            'value_course_grades_show_n': '不公布',
            'value_course_grades_show_p': '公布個人',
+            'value_course_grades_show_a': '公布全班',


這個 commit 只增加設定值而無實際使用，是不是應該把「公布全班」相關的修改移來這個 commit，或是把這個 commit 併入下個 commit？理想上我們會希望每個 commit 都是一個完整、有意義、可以測試的修改。

lantw44 · 2022-01-31T18:53:42Z

ceiba_dl/vfs.py

@@ -353,6 +353,8 @@ def _create_course_list_map(self):
        course_list_page = self.vfs.request.web('/student/index.php')
        course_list_rows_all = course_list_page.xpath('//table[1]/tr')
        course_list_rows = course_list_rows_all[1:]
+        # Add 旁聽 courses
+        course_list_rows += course_list_page.xpath('//table[2]/tr')[1:]


如同我在 #13 (comment) 所說，這樣的修改對不曾旁聽過任何課程的學生會有問題。

lantw44 · 2022-01-31T18:59:04Z

ceiba_dl/vfs.py

@@ -641,144 +643,160 @@ def fetch(self):
        assert len(student_page.xpath('//table')) > 0

        student_rows = student_page.xpath('//div[@id="sect_cont"]/table/tr')
-        assert len(student_rows) == 12
+        # NTNU and NTUST students may have less rows
+        #assert len(student_rows) == 12


我其實有點好奇你遇到的是什麼狀況，不知道能不能貼個截圖（去除個資）當作參考，同時也讓之後看到這個 pull request 的人能更了解這個問題？我目前看過的 ntnu_* 和 ntust_* 帳號都和普通的臺大帳號一樣有 12 列，不知道是不是只有特定帳號有這個情形。

lantw44 · 2022-01-31T19:14:48Z

ceiba_dl/vfs.py

@@ -795,7 +813,8 @@ def fetch(self):
        assert set(result.keys()) == set(result_keys)

        days = '一二三四五六日'
-        slots = '01234@56789XABCD' # 節次 (possible time slots: See https://nol.ntu.edu.tw/nol/guest/index.php for more information)
+        #slots = '01234@56789XABCD' # 節次 (possible time slots: See https://nol.ntu.edu.tw/nol/guest/index.php for more information)
+        slots = '01234@56789XABCD ' # For me, 暑期實習 has slot with space character


除非舊版的程式碼未來很可能會恢復使用，不然請不要用註解的方式刪除或修改程式碼。要刪就直接刪掉，要改也直接改掉，想看舊版的人可以自己去翻 git log。以這裡來說，你可以把舊的註解移到上面一行，再加上你這裡寫的新註解。註解中的「For me」看起來不太必要，其他人可能不看 git log 大概也不知道這裡的「me」是誰。

lantw44 · 2022-01-31T19:41:01Z

ceiba_dl/vfs.py

@@ -3121,7 +3146,8 @@ def __init__(self, vfs, parent, cell):
    def fetch(self):
        s = self.vfs.strings

-        assert not element_get_text(self._cell).strip()
+        # I have a course which only has it assistants' names in '課程助教' column with pure text
+        #assert not element_get_text(self._cell).strip()


同上面所說，不建議用註解的方式來刪除程式。另外以這裡來說，你應該會想要把名字記下來而不是直接忽略。

lantw44 · 2022-01-31T19:45:37Z

ceiba_dl/vfs.py

@@ -644,7 +644,7 @@ def fetch(self):

        student_rows = student_page.xpath('//div[@id="sect_cont"]/table/tr')
        # NTNU and NTUST students may have less rows
-        #assert len(student_rows) == 12
+        assert len(student_rows) <= 12


這看起來是個 fixup commit，應該直接併入上一個 commit。

b04505009 added 3 commits December 8, 2020 23:49

use pathvalidate to make the local file path is valid

6f9f504

add another possible attribute '公布全班'

4728d90

add support for 旁聽courses, 暑期實習timeslots, 學生from NTUST or NTNU, 成績公佈w…

5383f8c

…ith '公佈全班' attribute, and comment out some assertions base on my course list

b04505009 mentioned this pull request Dec 9, 2020

add filename validator before saving files #15

Open

ryucc reviewed Apr 5, 2021

View reviewed changes

Add assertion len(student_rows) <= 12

58f5bf5

lantw44 requested changes Jan 31, 2022

View reviewed changes

Merge branch 'lantw44:master' into master

915b6b4

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix some issues to work on my ceiba #16

Fix some issues to work on my ceiba #16

b04505009 commented Dec 9, 2020

ryucc Apr 5, 2021

b04505009 Apr 5, 2021

lantw44 Jan 31, 2022

lantw44 commented Jan 31, 2022

lantw44 Jan 31, 2022

lantw44 Jan 31, 2022

lantw44 Jan 31, 2022

lantw44 Jan 31, 2022

lantw44 Jan 31, 2022

lantw44 Jan 31, 2022

lantw44 Jan 31, 2022

lantw44 Jan 31, 2022

lantw44 Jan 31, 2022

Fix some issues to work on my ceiba #16

Are you sure you want to change the base?

Fix some issues to work on my ceiba #16

Conversation

b04505009 commented Dec 9, 2020

New features

Unexpected conditions on my ceiba

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lantw44 commented Jan 31, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment