search

r0oth3x49 / udemy-dl

A cross-platform python based utility to download courses from udemy for personal offline use.
MIT License
4.85k stars 1.19k forks source link

Remove invalid Windows characters #55

Closed ipkpjersi closed 6 years ago

ipkpjersi commented 6 years ago

Hello,

I found another issue with this program. On Ubuntu, it downloads folder names and file names as they are and doesn't check if there are any invalid Windows characters. For an example of characters I am talking about, check this link: https://stackoverflow.com/questions/1976007/what-characters-are-forbidden-in-windows-and-linux-directory-names

Basically, I feel that this program should replace the invalid Windows characters with a different character, or with nothing else.

Thank you.

r0oth3x49 commented 6 years ago

@ipkpjersi it already cleans up that for windows have a look at this issue #29 and #41. this udemy-dl and the udemy-dl on PYPE (a python repo) both are different if you are using that one then that's not the one which is here. i assume you are not mixing both.

ipkpjersi commented 6 years ago

It did not work for me, at least for folder names: https://i.imgur.com/WhTjK6U.png

As you can see, it has : in the folder name which as far as I know, is one of the invalid Windows characters.

I have downloaded the master branch of this repository and am running it from that. The one on PYPE (which seems to be different) does not have this issue with the : character.

r0oth3x49 commented 6 years ago

@ipkpjersi can you please clone the latest repo available here on github and try once?

ipkpjersi commented 6 years ago

I have just spotted the issue. You only remove the special Windows characters if the OS is Windows, but I want it removed regardless that way if you download videos on Mac or on Linux, you can still use them all just fine on Windows too. I think this would be a great enhancement for this program. I removed if os.name == "nt": from extractor and downloader Python files, and now it works as expected. I suggest implementing this in the master branch, I can open a PR if you would like.

r0oth3x49 commented 6 years ago

@ipkpjersi this is not the issue for the 1st because i am skipping this part for Mac and Linux intentionally as both of these Operating Systems don't have a problem with naming convention. Thanks for suggestion i am taking this suggestion as future enhancement. for now i think this is not the problem for many users.

Update i 'm closing this issue as i said i will consider this for future enhancement.