Logged-In As
ACCOUNT
Not Logged In
Use OI Filemanager with OI Safe OpenIntents
Status: Closed Time to complete: 336 hrs Mentors: Peli, Randy McEoin, Friedger Müffke, Manuel R. Ciosici Tags: Android, Java

The task is to add support for OI File Manager within OI Safe for backup, restore, export, and import of OI Safe data. See also issue 186.

You should use the distribution library to reuse existing code (ask to download file manager if it is not present, ...).

Your solution should consist of a patch (instructions) based on the latest version of OI Safe. If you don't have it yet, you first need to download and install the Android SDK (instructions). Follow these instructions to build OI applications.

  • Difficulty: medium
  • Time limit: 14 days
  • Prerequisites: Download the Android SDK (instructions)
  • Requirements for completed entry:
    • Send in a patch with your code modifications (instructions).
    • The code must follow our StyleGuide.
    • OI Safe must be able to use OI File Manager to specify locations for backup, restore, export, and import.
Uploaded Work
File name/URL File size Date submitted
http://gci.shulik.cz/safe/choose-file.patch n/a December 17 2011 19:49 UTC
http://gci.shulik.cz/safe/choose-file-3.patch n/a December 18 2011 13:30 UTC
http://gci.shulik.cz/safe/choose-file-4.patch n/a December 19 2011 21:51 UTC
http://gci.shulik.cz/safe/choose-file-5.patch n/a December 20 2011 06:07 UTC
http://gci.shulik.cz/safe/choose-file-6.patch n/a December 22 2011 17:28 UTC
http://gci.shulik.cz/safe/choose-file-tabs... n/a December 22 2011 18:35 UTC
Comments
Matěj Konečný on December 17 2011 08:32 UTC Task Claimed

I would like to work on this task.

Peli on December 17 2011 19:37 UTC Task Assigned

This task has been assigned to Matěj Konečný. You have 336 hours to complete this task, good luck!

Matěj Konečný on December 17 2011 19:49 UTC Ready for review

The work on this task is ready to be reviewed.

Matěj Konečný on December 17 2011 19:50 UTC ...

I had a question here, in this patch are strings suggested by Peli.

Peli on December 18 2011 01:16 UTC NPE

Menu > Backup: OI File Manager opens.


Now press "Back" -> NullPointerException:


 


12-18 02:11:13.769: E/AndroidRuntime(24285): FATAL EXCEPTION: main


12-18 02:11:13.769: E/AndroidRuntime(24285): java.lang.RuntimeException: Failure delivering result ResultInfo{who=null, request=7, result=0, data=null} to activity {org.openintents.safe/org.openintents.safe.CategoryList}: java.lang.NullPointerException


[...]


12-18 02:11:13.769: E/AndroidRuntime(24285): Caused by: java.lang.NullPointerException


12-18 02:11:13.769: E/AndroidRuntime(24285):at org.openintents.safe.CategoryList.onRequestImportFilename(CategoryList.java:794)


12-18 02:11:13.769: E/AndroidRuntime(24285):at org.openintents.safe.CategoryList.onActivityResult(CategoryList.java:786)


[...]

Peli on December 18 2011 01:16 UTC Task Needs More Work

One of the mentors has sent this task back for more work. Talk to the mentor(s) assigned to this task to satisfy the requirements needed to complete this task, submit your work again and mark the task as complete once you re-submit your work.

Matěj Konečný on December 18 2011 09:38 UTC Ready for review

The work on this task is ready to be reviewed.

Matěj Konečný on December 18 2011 09:39 UTC Oops

Sorry, I was so confused by the previous onActivityResult, that I completely forgot, how to deal with that method. It's fixed now and re-uploaded to the previous location.

Peli on December 18 2011 10:55 UTC Auto-backup

I have a question:


* How do you deal with auto-backup?


And a suggestion:


* Currently the file name is not remembered anywhere, so if the user wants to store the backup in another location, they have to select this new location every time.


Would it be possible to introduce 2 new settings:


* Backup location


* Export location


that store the file paths? Auto-backup could use the same backup location from the settings.

Matěj Konečný on December 18 2011 10:57 UTC Settings

Great idea!

Peli on December 18 2011 11:18 UTC Task Needs More Work

One of the mentors has sent this task back for more work. Talk to the mentor(s) assigned to this task to satisfy the requirements needed to complete this task, submit your work again and mark the task as complete once you re-submit your work.

Peli on December 18 2011 12:20 UTC One more issue

If OI File Manager is not installed, OI Safe currently crashes on backup.


It should prompt for downloading OI File Manager (there is a default dialog in DistributionLibrary - see DistributionDemo). (It could additionally also backup to the default location).

Matěj Konečný on December 18 2011 13:31 UTC Ready for review

The work on this task is ready to be reviewed.

Matěj Konečný on December 18 2011 13:36 UTC Fixed

I hope everything is fixed. If OI File Manager not present, it backups to the default location. Please, try the autobackup - I'm quite sure it works, but I wasn't able to test it. The new file is the http://gci.shulik.cz/safe/choose-file-3.patch.


By the way, I downloaded the OI File Manager from the Market and it looks terrible. Although the icon issue is probably fixed (for .xml file it shows icon "Text" instead of folder icon), the new icons are huge.
Oh, I see. It uses icons of the apps that would open the file. That's great, but some of the icons are really huge.
Is it known issue or should I add it to the issue tracker?

Peli on December 18 2011 23:29 UTC Review

This looks very good already. A few minor issues:


* I think it is preferable to have a consistent user interface. If you ask for a file location for import and restore, you should also ask for a file location for export and backup. Some users may want to back up to a new file name each time.


The file name in the settings should be updated whenever the user provides a new file name for import, export, backup, or restore.


(If one really wanted to introduce a shortcut, this should be a separate setting "Always use the same location and don't ask".)


* To maintain maximum backward compatibility, I would not change the default file names. Keep them "oisafe.xml" and "oisafe.csv". (Otherwise, users without OI File Manager will not be able to restore after an upgrade)


* I know the code has really bad indentation (mixing tabs and spaces), but your patch should if possible not introduce more unnecessary spaces. (make them visible in your editor or in your diff viewer).


* If OI File Manager is not present and one clicks in the settings, a toast appears. The toast contains too much text so that one can not read it in time, which causes a feeling of slight stress. It would be better to combine the text in the dialog and the text in the toast, and put a shortened version of both into the dialog. 


Regarding the issue: There is a related issue 322, but you may add a new one for the icons in the list of files.


(There is another issue that could be added to the issues: Select import database > press OK in OI File Manger ... "Import" dialog appears ("Do you want to replace..."). Change screen orientation -> the dialog is gone.)

Peli on December 18 2011 23:29 UTC Task Needs More Work

One of the mentors has sent this task back for more work. Talk to the mentor(s) assigned to this task to satisfy the requirements needed to complete this task, submit your work again and mark the task as complete once you re-submit your work.

Matěj Konečný on December 19 2011 21:49 UTC Hope finally fixed

The app now asks the user also for the backup and export path - it was a great idea, it really is more consistent. If OI File Manager not present, it doesn't offer the user to download it - while there is no option to disable asking for downloading the app (maybe a feature request for the distribution library? To add a "Not ask me again" option to the download dialog, which would be remembered for e.g. 14 days.), if the user didn't want to download it, the dialog would disturb him and it wouldn't probably be felt as a great feature..


The filename settings are not updated, I personally would hate the behaviour. For example, if I were backing up to a files named safe_backup_1.csv, safe_backup_2.csv etc., I would set the default location to safe_backup_.csv and manually add just the number. If it updated the default path automatically, it wouldn't be possible.


The default filenamesnames are now changed back to the previous (not-too-much-saying) ones.


Indentation probably fixed (replaced spaces for tabs in gEdit), but it really needs a re-indentation patch. I'm not able to do it, I'm sleeping or going to bed, when you're here and the patch needs quick upload.


The toast was shortened. I think it's okay now. If I wanted to add the text to the dialog, I believe, I would have to duplicate some code and info from the Distribution Library, which I would hate to. Maybe another feature request to allow to add custom sentences to the DownloadOIAppDialog..


The new file is the http://gci.shulik.cz/safe/choose-file-4.patch.


So how about it? Is it OK now?

Matěj Konečný on December 19 2011 21:51 UTC Mistake

Oops, I copied the link from the uploaded work, but I copied link for patch 3. I changed the link description but forgot to change the link location. Here is the corrected link: http://gci.shulik.cz/safe/choose-file-4.patch

Matěj Konečný on December 19 2011 21:51 UTC Ready for review

The work on this task is ready to be reviewed.

Peli on December 20 2011 01:17 UTC Save name to settings

The filename settings are not updated, I personally would hate the behaviour. For example, if I were backing up to a files named safe_backup_1.csv, safe_backup_2.csv etc., I would set the default location to safe_backup_.csv and manually add just the number. If it updated the default path automatically, it wouldn't be possible.


Let's assume there are 2 kinds of users:


* Those who don't find out about settings, and navigate to a location and enter a file name every time they do a backup.


* And those who find out about settings, and set a clever default location.


If the modified path is stored every time, then users of the first group will have a much simpler time, and users of the second group only have slightly more effort (delete one character, and add another). Plus, they will know which file name they used last time (was it "_34" or "_35"?) without scrolling down to the bottom of the list. If it is a large number, they even save time (enter "123", "124", "125", vs. delete "3" and add "4")


So I still think it is better to update the settings on every successful backup or restore operation.

Matěj Konečný on December 20 2011 06:08 UTC Implemented

Though I don't like it very much, I implemented it. You have bigger experience with users. http://gci.shulik.cz/safe/choose-file-5.patch

Randy McEoin on December 22 2011 02:27 UTC patch file format issues

A large portion of the patch fails for me.  When I do the following in Eclipse, OI Safe -> Team -> Apply Patch -> find the patch file.   It shows a bunch of unmatched Hunks.   When I compare the failed hunks with the original file, I see that the original file is formated with spaces, but the patch hunk thinks the original lines had tabs.


Can you try the following?



  1. Close the "OI Safe" project.

  2. Move your "Safe" folder to somewhere else.   Make sure you don't mistakenly lose this.   You have a bunch of good work in there.

  3. Do a "svn update" to get the current version of OI Safe.

  4. Import the SVN version of the OI Safe.  Now you should have exactly what I have.

  5. Try to apply your patch to the SVN version of the project.


I use Ubuntu and normally I use the command line "svn" to do the various tasks like "svn diff", "svn status" and so forth.   For patching I normally do a "patch -p0 -i patchfile".   This time around I tried my luck with the built-in Eclipse Team -> Apply Patch.   Seemed a little handier to visually see the space/tab mismatch.


Anyway, please try out reverting back to the SVN repository version of OI Safe and applying your patch.   If it simply works, then perhaps there is something else I need to do.   If it fails, then you'll see what I'm seeing.


The patch looks really good and it'll be great to have this incorporated.   


I've seen this issue with some others and I never found out what changed to get the spaces/tabs to match and create a useable patch.   If you do end up making a change to how you do the patch or Eclipse or something, please do share.

Randy McEoin on December 22 2011 02:27 UTC Task Needs More Work

One of the mentors has sent this task back for more work. Talk to the mentor(s) assigned to this task to satisfy the requirements needed to complete this task, submit your work again and mark the task as complete once you re-submit your work.

Matěj Konečný on December 22 2011 17:27 UTC Problem

I'm sorry, it was my fault, more precisely fault of my inexperience with SVN and weak intelligence - as Peli wrote, I replaced spaces for tabs in the patch file. Very smartly, I thought. I opened the patch file and run three regular expression text substitusion. But I didn't realized, that I changed also the current lines of code. http://gci.shulik.cz/safe/choose-file-6.patch is a working patch, with space indentation. I'll try to fix the indentation, but I'm not sure whether I'll succeed. I think a reindentation patch would be much better and easier..

Matěj Konečný on December 22 2011 17:31 UTC Ready for review

The work on this task is ready to be reviewed.

Matěj Konečný on December 22 2011 18:33 UTC Problem found

I found all the problems: In the patch, that didn't work for you were spaces changed to tabs. In the base patch, where the spaces weren't substituted, was a block, that shouldn't be there. It was a block of imports, it didn't change anything, just put the imports in the alphabetical order. But as I updated to the new version od OI Safe, the imports were probably already sorted and this is what was causing the problem.


According to the space-tab problem: The only correct solution is a complete reindentation. I personally suggest wait till the end of GCI and then, without tons of people working on the code, reindent it. But while Peli required me to reindent the code, I came up with a little CoffeeScript program, that substitutes the spaces with tabs, but only on lines beginning with +. The window.tablength means how many spaces are there for one tab. If you wanted to, feel free to use it.


 


 


window.regex = /^(\+)( +)(.*)$/gm
window.tablength = 4
window.times = (times, char) ->
   (char for i in [0...times]).join ""
window.tabify = (spaces) ->
   len = spaces.length
   if len < tablength
      return spaces
   rem = len % tablength
   len = len - rem
   times(len/tablength, "\t")+times(rem, " ")
window.process = (text) ->
   text.replace regex, (ignore, a, b, c) ->
      a + tabify(b) + c


window.text = """


the patch


"""

Matěj Konečný on December 22 2011 18:35 UTC The file with tabs

http://gci.shulik.cz/safe/choose-file-tabs.patch

Randy McEoin on December 23 2011 16:42 UTC Commit

That patch file worked great for me from within Eclipse.  


Committed the patch as revision 3916.


http://code.google.com/p/openintents/source/detail?r=3916

Randy McEoin on December 23 2011 16:42 UTC Task Closed

Congratulations, this task has been completed successfully.

Peli on December 23 2011 23:02 UTC CoffeeScript

I've added your CoffeeScript to our wiki:


http://code.google.com/p/openintents/wiki/PatchSubmission


If you have short instructions how to use and apply this script, this would be helpful.

Matěj Konečný on December 26 2011 17:18 UTC CoffeeScript

Sorry for the three-day delay - I always read the e-mails on my phone and I forgot to write here. As an apology, I made an ugly simple webpage that should solve the problem. It's http://tabify.shulik.cz/. I tested it in Chrome, it works there. I hope, that it edits the patch correctly, couldn't test it yer, my ADB is updating hence I can't use Eclipse. Feel free to contact me, to send the link to anyone or to fork the code and to anything with it.

Peli on December 27 2011 10:05 UTC Thanks

I've added a link to your site here:


http://code.google.com/p/openintents/wiki/PatchSubmission#Clean_up_patches

Matěj Konečný on December 27 2011 10:11 UTC Great

Hope it will help someone. By the way, the CoffeeScript under the isn't complete. The two last lines are missing.


 window.text = """
+  the patch
+"""

Peli on December 27 2011 10:25 UTC Thanks

Script updated. I hope it is fine now.

Matěj Konečný on December 27 2011 10:27 UTC It is

Yes, it should be okay.