-
Notifications
You must be signed in to change notification settings - Fork 75
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add string format and os.path.join #48
base: master
Are you sure you want to change the base?
Conversation
@GBLin5566 Seems like there are some build errors and conflicts too. These might have been solved by the recent merges. Can you rebase from master and push again? |
Hi @rohithasrk the conflict and testing are fixed now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! Just some minor issues.
Also, you missed some :) Example: https://github.com/GBLin5566/django-filemanager/blob/5f3d7184db02185ee6c6b963ab480df67cdbebc8/filemanager/__init__.py#L143
Finally, when looking at your changes, I noticed a lot of places where os.path.join
would really be more appropriate. I personally think that would go beyond the original goals of this issue though, so feel free to file a new issue.
filemanager/__init__.py
Outdated
else: | ||
string1 = 'Folder couldn\' be created ' | ||
string2 = 'because maximum number of folders exceeded : ' |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
@@ -184,7 +184,28 @@ function getPosition(e) { | |||
|
|||
function rightclick_handle(e,id,type) | |||
{ var c = getPosition(e); | |||
if(type == 'dir'){ | |||
if(type == 'dom'){ |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
@@ -48,10 +48,21 @@ | |||
<div id="message"> | |||
</div> | |||
</div> | |||
<div id="content"> | |||
<div id="content" onmousedown='rightclick_handle(event,dir_id,"dom");'> |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
.travis.yml
Outdated
@@ -26,5 +26,5 @@ before_script: | |||
script: | |||
- coverage run --source==filemanager runtests.py | |||
|
|||
after_success: | |||
coveralls | |||
# after_success: |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ErwinJunge The HTML and JS changes are already merged in the repo. Seems like changes from PR #49. They shouldn't actually reflect in this commit though.
@ErwinJunge @rohithasrk The HTML and JS files are commited due to the wrong way doing rebase. It should be fixed now, sorry for the mistakes. |
@ErwinJunge I've checked it again. Please check it out, thanks. |
@GBLin5566 I think I only found 1 more, almost done 👍 https://github.com/GBLin5566/django-filemanager/blob/6fe98204ff7720448cbed6fbc9aa63e8feafba1e/filemanager/widgets.py#L41 |
This is a PR reference to #35 .