Skip to content
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

Problem removing unused dimens and strings #16

Closed
sb673 opened this issue Apr 18, 2015 · 11 comments
Closed

Problem removing unused dimens and strings #16

sb673 opened this issue Apr 18, 2015 · 11 comments
Assignees

Comments

@sb673
Copy link

sb673 commented Apr 18, 2015

When using the latest version of your script, the console output correctly prints out the message

removing ('dimen', "<dimen_name>") from resource ./res/values/dimens.xml
removing ('string', "<string_name>") from resource ./res/values/strings.xml

for all dimens and strings files but doesn't actually remove the said unused resource from the file. Is this a known issue?

@yhwang0916
Copy link

i meet this problem

@ca77y ca77y self-assigned this Apr 20, 2015
@ca77y
Copy link
Contributor

ca77y commented Apr 30, 2015

this is not correct. the second argument in the message should be the name of the resource ('dimen', 'missing_dimen')

if it's empty it means the name wasn't picked up correctly. could you paste the file which causes the problem

@sb673
Copy link
Author

sb673 commented Apr 30, 2015

I apologize, github formatting problems. The second argument wasn't an empty string but the id of the unused dimen. Sorry for the confusion. It was like this:

removing ('dimen', "<dimen_id_tag>") from resource ./res/values/dimens.xml
I unfortunately cannot paste the file.

@ca77y
Copy link
Contributor

ca77y commented May 1, 2015

i can't reproduce it. can you create a test for that in the sample app https://github.com/KeepSafe/android-resource-remover/blob/master/test/android_app/res/values/dimens.xml

@oneeyedhobbit
Copy link

I'm having a related issue, but with different results: when trying to remove unused string resources, the entire contents of the xml file are deleted, causing the script to error (presumably from reading an EOF byte instead of a string). Here's the traceback:

Namespace(app='.', ignore_layouts=False, lint='lint', xml='shareplan/build/outputs/lint-results.xml')
removing ('string', 'title_about`') from resource /Users/william.jarvis/Documents/Code42Development/c42_shareplan_android/shareplan/src/main/res/values/strings.xml
Traceback (most recent call last):
File "/usr/local/bin/android-resource-remover", line 9, in
load_entry_point('android-resource-remover==0.1.0', 'console_scripts', 'android-resource-remover')()
File "/usr/local/lib/python3.4/site-packages/android_clean_app.py", line 138, in main
remove_unused_resources(issues, app_dir, ignore_layouts)
File "/usr/local/lib/python3.4/site-packages/android_clean_app.py", line 132, in remove_unused_resources
remove_resource_value(issue, filepath)
File "/usr/local/lib/python3.4/site-packages/android_clean_app.py", line 120, in remove_resource_value
tree.write(resource, encoding='utf-8', xml_declaration=True)
File "lxml.etree.pyx", line 1947, in lxml.etree._ElementTree.write (src/lxml/lxml.etree.c:57536)
File "serializer.pxi", line 504, in lxml.etree._tofilelike (src/lxml/lxml.etree.c:122727)
File "lxml.etree.pyx", line 316, in lxml.etree._ExceptionContext._raise_if_stored (src/lxml/lxml.etree.c:10323)
File "serializer.pxi", line 415, in lxml.etree._FilelikeWriter.write (src/lxml/lxml.etree.c:121463)
TypeError: must be str, not bytes

@benjamin-bader
Copy link

@oneeyedhobbit Thanks for the report; can you please open a separate issue for this? It will help us keep track of what's still open and what is fixed.

@oneeyedhobbit
Copy link

@benjamin-bader actually when I clone the repo and run the script directly everything seems to be working fine. It might just be that the version installed via pip is out of date. In light of that, would you still like me to open a new issue?

@benjamin-bader
Copy link

Glad to hear that things are working again! I'd say it's up to you whether to open a new issue. If you find that there is a problem, please feel free to open one.

@oneeyedhobbit
Copy link

Nope, everything is great running the script from the repo. You may want to remove the instructions to install via pip or (update the version you have uploaded there). Thanks for the awesome package =)!

@ca77y
Copy link
Contributor

ca77y commented May 1, 2015

@oneeyedhobbit the pip version is indeed old and we will be updating it. thx for feedback

@ca77y
Copy link
Contributor

ca77y commented May 21, 2015

pip updated

@ca77y ca77y closed this as completed May 21, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants