Fixes to randomtext #1
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Conflicts: Readme.md
Made %HOME% the default install location for rsshell in Windows. This
is not a system install as intended but at least it works. A complete
solution in the future might require a full-blown MSI installer.
A working system installer for Windows was almost completed, but I
wasn't able to solve a problem with file/directory permissions. I was
running setup.py from an administrator account, and the script was
apparently sufficiently privileged to write to %PROGRAMFILES% since
the check on line 144 didn't fire. However, on line 189 the program
was interrupted with an IOError. Somehow, having write permissions to
a directory in Windows doesn't mean that one can create
subdirectories.
I tried manually creating the subdirectory (%PROGRAMFILES%\Red Spider
Project) and then running the script again, but it would just get
stuck on the next creation at line 191. I've attached the traceback at
the end of this commit message.
I've created the branch 'jgonggrijp/setup-windows' so that the old
state of setup.py can be permanently accessed. Note that the line
numbers in my explanation above, as well as those in the traceback
below, refer to the state in jgonggrijp/setup-windows, not to the
state in the present commit.
The install_rsshell_windows function has been moved past the
invocation of main.
Traceback (most recent call last):
File "C:\Users\Julian\Documents\RedSpider\setup.py", line 314, in <module>
main()
File "C:\Users\Julian\Documents\RedSpider\setup.py", line 53, in main
install()
File "C:\Users\Julian\Documents\RedSpider\setup.py", line 99, in install
install_rsshell()
File "C:\Users\Julian\Documents\RedSpider\setup.py", line 132, in install_rsshell
install_rsshell_windows(src_file, fname)
File "C:\Users\Julian\Documents\RedSpider\setup.py", line 166, in install_rsshell_windows
rsshell_install_finish(src_file, rsshell_target_dir, fname)
File "C:\Users\Julian\Documents\RedSpider\setup.py", line 191, in rsshell_install_finish
copy2(src_file, bin_file)
File "C:\Python27\lib\shutil.py", line 128, in copy2
copyfile(src, dst)
File "C:\Python27\lib\shutil.py", line 83, in copyfile
with open(dst, 'wb') as fdst:
IOError: [Errno 13] Permission denied: 'C:\\Program Files\\Red Spider Project\\rsshell.py'
While considering the option to install rsshell internally in the project root on Windows (instead of system-wide), I realized that this would be the best option anyway for all platforms. It improves consistency across platforms, it simplifies the program logic of setup.py, it removes the need to run setup.py with administrative rights and it allows multiple users on the same computer to install different version of rsshell. The only real difference with previous versions on the master branch is that a special subdirectory is reserved for externally visible executables: extbin, with a matching extension of the user's PATH settings. This directory has also been added to the .gitignore file.
We're appending /extbin to the user's PATH anyway, so it makes sense to do the same thing with RED_SPIDER_ROOT. This removes the need to create ~/.config/xkcdRedSpider (POSIX) or %APPDATA%\xkcdRedSpider (Windows). From now on, we'll need to modify only one location that is not within the project root: ~/.profile on POSIX or HKEY_CURRENT_USER in the Registry on Windows.
Update shebangs
There are two reasons to merge upstream into topic:
1. The changes on topic warrant an update of Readme.md, which is
outdated.
2. Since master on upstream and local have diverged for so long, it's
a safe testdrive for merge conflicts.
Merge rsroot-env into master Several changes to the setup script and rsshell that have been impatient to make their way into the master branch have finally been tested and made ready for the merge. In particular the logic for detecting the project root directory has been moved entirely to setup.py and the new internal subdirectory /extbin is added permanently to the user's PATH so that rsshell can be invoked at any time from any working directory. Since a lot happened since the branch forked from master, Readme.md has also been updated.
rsshell "teleports" to ROOT
Conflicts: setup.py src/rsshell.py
Just set the executable bit for user, group, and other.
Don't overwrite the file's mode completely.
Fixed setup.py tab/space mixing.
Make random-number python3 compatible.
This fix makes the output also look a lot more like natural language.
|
Thanks for the quick response. :) |
|
No problem. Sorry if I wasn't as active as I would like. Anyways, I'm keeping tabs on the project and if anything is necessary I will respond. Other than that, as much as I can I will try to contribute to the project :) |
|
It was a pleasure! No need to apologize. :) Finishing randomtext and randomascii is sort of necessary of course. Otherwise, why did you start working on it? That said, we don't have a time limit. With my changes rtext might already be very close to finished (for the time being, until you or somebody else decides they want to continue hacking on it). I think you can just do a pull request for it (to master on the central repo) straight away. You're always very welcome to come around with a contribution, even if it's just a one-liner in an issue ticket. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I thought it was time to dust off some old abandoned commands so I started with
randomtext. I think it's a very interesting program. Hopefully this pull request will bring it closer to integration with the rest of the project. :)Please don't be intimidated by the long lists of commits and changed files. Most of that is just a side effect of me pulling in the integration branch to update everything that has changed in the meanwhile. Only the commits of today (November 3rd, at the bottom of the list) and the changes in
randomtext.pyare truly relevant. Also note that the diff forrandomtext.pylooks like I replaced all code, but this is not the case; it looks like that because I changed the line endings. The commits 7620948, 1cd2563, 3346b44 and 0e7e7f0 show what I actually did.Summary of changes:
_usageintousage(without underscore).setup.pyandrsshellas well as several commands that have appeared in the meanwhile, such asrshelpandlevel_up.randomtextwithsetup.pyso it will be installed as part of the red spider package andrshelpwill be aware of its usage line.rsshellcontext as justrandomtextrather than the full-blownpython randomtext.py.Please accept these changes as a courtesy. :)