Save user-selected-path to configuration file. about ldr-importer HOT 18 CLOSED

I have gone through and reworked the code to be less hack-ish in terms of file paths, but I still need to add it back to the script and test it.

from ldr-importer.

I should probably remove the execution/importing hack and replace it with something that reads the file and uses a regex to get the right variables.

No time for a couple of days though.

from ldr-importer.

Alright. You could try that if you want. I'll test out my improvement of the code hopefully tomorrow and display the changes, and you can decide what do to. A few days is fine, you do not have to work on it 24/7. Besides, I think Banbury was going to try to work on it this weekend, and we need to try to avoid any conflicts. All my code is pushed, so I'm done for now

from ldr-importer.

https://gist.github.com/le717/8659933f479f6bc22a7e

My changed code, plus a few comments. It works on Windows. The only thing left (assuming it still works for you) is to hook it into the auto-detection for Windows from #15.

Two things:

1. Islinux required in the dictionary? .get(sys.platform, LinuxLDrawDir) does that for us, and since it is recommended to use .startswith() to detect Linux, .get() helps us gracefully detect it.
2. Always use with when opening files (read or write, text or binary). I explained this all in my aptly named Python tutorial, with. 😉

from ldr-importer.

1. No, I just changed it that way for a bit to make it easier to add the user dir variable. All of this code is just a sketch/proof of concept.

Thank you for the info on linux detection!

1. Good to know, thanks a lot.

I really have a lot to learn in the area of knowing what is considered proper, I normally just do what seems to work. I can see that this project will help me learn a lot! (Hopefully I can still be of service to it too.) :3

from ldr-importer.

I'm glad you are you learning, and that we can be of assistant to you. If you have a question or are confused, just ask. ;)

Ah, proof of concept code. Makes more sense now.

Yes, you can still be of service. We all have to start somewhere.

BTW, you can now push directly to this repo, no more reverse PR. Welcome to the team, Tribex. 😄

If you have the time, I've pushed the cleaned code to the save_install_path branch. Perhaps you can tell what is going on (the Windows detection seems to work, but the config file is not written).

from ldr-importer.

Welcome to the team, Tribex. 😄

Thanks! I'll try to be a help, not a burden.

I've pushed the cleaned code to the save_install_path branch. Perhaps you can tell what is going on (the Windows detection seems to work, but the config file is not written).

Will take a look at it when I have time, just got back from church.

UPDATE (Fixed):
(https://github.com/Tribex/Blender-2.6-LDraw-Importer/commit/be65d9dce37b2776611c84b0797633331c979340)
Okay, I found the problem. The issue is twofold and stems from the same thing.

How it works: The config file is read as a normal python file, and is basically used like this:

from config(.cfg) import *

The reason it wasn't working is because there are two improperly-commented lines written to the config.cfg, so the main script is unable to read it as python.

After that, you will have a variable called ldraw_dir with the value of what was in the config file. That needs to be assigned to the selection menu.

One other thing, Shouldn't we name config.cfg something more unique? Or maybe move it into a sub-folder? Because it is possible that other Blender addons might write a config.cfg, in which case it will be in the same folder as our config.cfg (overwriting it), which would cause addon compatibility issues

from ldr-importer.

https://github.com/le717/Blender-2.6-LDraw-Importer/tree/save_install_path

Newest work there. So far, it works with the detection except for..

If the path contains \t, \n, \r, or the like, it is interpreted rather
than treated like a raw string, as it should be (example:
C:\temp\LDraw). I added 'r' to each string and converted all single
backslashes to os.path.sep, but it still doesn't work. (message from 7f57471)

Perhaps you can figure this out. I'm stumped.

As for pushing straight to here, simply git clone this repo and git checkout to the save_install_path branch. Tada!

One other thing, Shouldn't we name config.cfg something more unique? Or maybe move it into a sub-folder? Because it is possible that other Blender addons might write a config.cfg, in which case it will be in the same folder as our config.cfg (overwriting it), which would cause addon compatibility issues

I renamed it to a .py so we can simply use from config import ldraw_dir and remove the "hack", but it can still be changed. Subfolder? No issue. Run setup.py and look in the Archives folder. That is how a release is made, and it is all in a subfolder.

The reason it wasn't working is because there are two improperly-commented lines written to the config.cfg, so the main script is unable to read it as python.

Oops, my fault. Didn't realize it would treat it like a normal python script (that's why I changed the # to a //). My bad.

from ldr-importer.

I know how to fix the raw strings. I was making it raw in every place but where it was really needed. facepalm

from ldr-importer.

XD
Glad you were able to fix it! What else is needed before it will be ready to be merged (if everyone likes it)?

I'm currently working on my website/CMS, a bit busy to work on this ATM.

from ldr-importer.

I still have to fix it. I know how to to, but I have to turn on my laptop first (or use the website).
Testing on Mac OS X, testing on Linux, removing commented code after those are tested, fixing them if there is an issue. Then we can merge this.

from ldr-importer.

Okay, was just wondering.

By the way, I have Git installed on my laptop now, so hopefully there will be less mess-ups.

from ldr-importer.

It seems like a lot but it is really not. 😉

If you get a minute break from your website design, Linux needs testing. My Linux VM is suddenly broken, and the native Blender port will not run. :\

Have fun with your website design!

from ldr-importer.

Sure, testing now.

EDIT: Nope, not working, will look into it as soon as I finalize my CMS.

Traceback (most recent call last):
  File ".config/blender/2.69/scripts/addons/ldraw_import.py", line 954, in RunMe
    saveInstallPath(self)
  File ".config/blender/2.69/scripts/addons/ldraw_import.py", line 1082, in saveInstallPath
    ldpath = r"{0}".format(ldpath)
UnboundLocalError: local variable 'ldpath' referenced before assignment
File ".config/blender/2.69/scripts/addons/ldraw_import.py", line 952, in RunMe

Error appears when changing the value in the preset list or editing the text field.

from ldr-importer.

Aha, sorry about that. That was my fault. Since I only run Windows I mainly target Windows (though I do keep cross-platform in mind) and what happened was one of my Windows-only conditions. Fixed it (and the raw strings!) in commit ce090ea.

from ldr-importer.

Ok, it works now. :D

from ldr-importer.

Closed in commit 33edd81.

from ldr-importer.

@Banbury While you are on, will you quickly check if the config file stuff from the newest commit works? I'm the Windows dev with no way to test on Mac (and my Linux VM is broken), so I am not sure if it will work or not.

from ldr-importer.