coala_html.py: Bug in os.walk approach to detect empty directory #83
Assignees
Labels
Comments
|
Thanks for reporting this issue! @coala-analyzer/coala-contributors, your aid is required, fellow coalaian. Help us triage and solving this issue! |
tushar-rishav
added a commit
that referenced
this issue
Aug 5, 2016
The current approach depends on `os.walk` to get the content for a directory and then determine if the directory is empty. The current approach is bugy in implementation and also the approach isn't the best approach as it requires traversing the directories recursively just to make sure that parent directory is not empty. The new approach with this commit, does this jon by checking for the existence of `index.html` in target directory. Since we already know the structure of target directory which happens to be an angular package, we can just check if the `index.html` exists in the target directory. Fixes #83
tushar-rishav
added a commit
that referenced
this issue
Aug 5, 2016
The current approach depends on `os.walk` to get the content for a directory and then determines if the directory is empty. The current approach is bugy in implementation and also the approach isn't the best approach as it requires traversing the directories recursively just to make sure that parent directory is not empty. The new approach with this commit, does the job by checking for the existence of `index.html` in target directory. Since we already know the structure of target directory which happens to be a known angular package, we can just check if the `index.html` exists in the target directory. Fixes #83
tushar-rishav
added a commit
that referenced
this issue
Aug 6, 2016
The current approach depends on `os.walk` to get the content for a directory and then determines if the directory is empty. The current approach is bugy in implementation and also the approach isn't the best approach as it requires traversing the directories recursively just to make sure that parent directory is not empty. The new approach with this commit, does the job by checking for the existence of `index.html` in target directory. Since we already know the structure of target directory which happens to be a known angular package, we can just check if the `index.html` exists in the target directory. Fixes #83
tushar-rishav
added a commit
that referenced
this issue
Aug 6, 2016
The current approach depends on `os.walk` to get the content for a directory and then determines if the directory is empty. The current approach is bugy in implementation and also the approach isn't the best approach as it requires traversing the directories recursively just to make sure that parent directory is not empty. The new approach with this commit, does the job by matching the directory contents. Moreover, user may choose to create a fork of the angular app and just update the json configs. This enables user to hack on the angular package but at the same time also forbids them to fetch the latest changes from main package. So it would be nice to detect if the local copy and the remote differs, if they do then ask user if they want to reload the changes from remote or skip it. In latter case the local copy would remain intact. To facilitate this a new `reload` cli argument is introduced. If user wants to fetch the changes from remote then he must pass the `--reload` flag. By default it is False i.e the local copy won't be overwritten with latest updates if any. Fixes #83
tushar-rishav
added a commit
that referenced
this issue
Aug 6, 2016
The current approach depends on `os.walk` to get the content for a directory and then determines if the directory is empty. The current approach is bugy in implementation and also the approach isn't the best approach as it requires traversing the directories recursively just to make sure that parent directory is not empty. The new approach with this commit, does the job by matching the directory contents. Moreover, user may choose to create a fork of the angular app and just update the json configs. This enables user to hack on the angular package but at the same time also forbids them to fetch the latest changes from main package. So it would be nice to detect if the local copy and the remote differs, if they do then ask user if they want to reload the changes from remote or skip it. In latter case the local copy would remain intact. To facilitate this a new `reload` cli argument is introduced. If user wants to fetch the changes from remote then he must pass the `--reload` flag. By default it is False i.e the local copy won't be overwritten with latest updates if any. Fixes #83
tushar-rishav
added a commit
that referenced
this issue
Aug 6, 2016
tushar-rishav
added a commit
that referenced
this issue
Aug 6, 2016
tushar-rishav
added a commit
that referenced
this issue
Aug 6, 2016
tushar-rishav
added a commit
that referenced
this issue
Aug 6, 2016
tushar-rishav
added a commit
that referenced
this issue
Aug 6, 2016
tushar-rishav
added a commit
that referenced
this issue
Aug 7, 2016
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
At present we are detecting if a directory is an empty before copying the angular package at coala_html.py, L38. This approach is faulty as for an empty directory
len(list(os.walk(<empty_dir>)))will be 1.Instead, I have better solution.Since we already know the content of the target directory, we can just check if any particular file e.g-> Bad approach. See more at #84index.htmlis present or not. I believe this approach is more efficient than traversing the directories recursively.The text was updated successfully, but these errors were encountered: