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

Add support to custom gitignore files #29

Closed
wants to merge 7 commits into from
Closed

Add support to custom gitignore files #29

wants to merge 7 commits into from

Conversation

filipekiss
Copy link

This adds two new commands: joe add and joe remove. This way you can
add your own custom gitignore files to joe.

In case of adding a custom file already available in the default list,
the custom one will be used. This should work fine in Windows too, but I
wasn't able to test it. Any feedback is apreciated.

A new dependency was added: termcolor, for colored output.

Address #9

This adds two new commands: `joe add` and `joe remove`. This way you can
add your own custom gitignore files to joe.

In case of adding a custom file already available in the default list,
the custom one will be used. This should work fine in Windows too, but I
wasn't able to test it. Any feedback is apreciated.

A new dependency was added: termcolor, for colored output.

The README file still needs to be updated.
The previous commit introduced a bug where the ignores located in the
Global folder would not work (for example, `osx`). This fix this
problem.

README still not updated.
This will update the README with the USAGE for `add` and `remove`.
@karan
Copy link
Owner

karan commented Jan 23, 2015

Ok this will take a while to review and merge. Please make sure to pull recent changes and resolve the merge conflicts. Thanks for helping out @filipekiss

@karan karan mentioned this pull request Jan 23, 2015
@filipekiss
Copy link
Author

@karan just merged the master and pushed the changes and teste on my machine. Everything should be ok right now.

@filipekiss
Copy link
Author

I'm closing this for now. Just found a problem with it in Linux and filenames. I'll correct this and re-open the PR when fixed.

@filipekiss filipekiss closed this Jan 23, 2015
Since now we are using colored displays, we need an index to keep the
filename and the cases since in a Case-Sensitive system this would cause
us trouble. I've tested this in Mac OS X and Linux, the first one case
insensite and the second case sensitive. It's working as expected.
@filipekiss
Copy link
Author

Found out the issue wasn't with Linux itself and actually with Case-Sensitve systems. Since my list was indexing things using only the lowercase name, I changed the way we use the indexes so we still maintain the filename.

@filipekiss filipekiss reopened this Jan 23, 2015
@karan
Copy link
Owner

karan commented Feb 7, 2015

Can you revert the commit that bumps the version? I'd like to bundle up a few PR's and then bump the version. :)

Also, tons of merge conflicts. 😢

@karan karan added this to the v0.1.0 milestone Feb 7, 2015
'Linux' : os.path.join(_HOME, '.joe'),
'Windows' : os.path.join(_HOME, '.joe')
}
path = paths[PLATFORM] or None
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think what you're looking for here is path = paths.get(PLATFORM) to prevent a KeyError.

However, I don't think this is necessary. What's wrong with just path = os.path.join(_HOME, '.joe')? If I ran this in a Java Virtual Machine, or some machine that may return something not in paths, it would be marked as unsupported when that command should always work fine (unless there's some deep problem, in which case python would just raise an exception).

@filipekiss
Copy link
Author

@karin no problem. I'll also try to merge the master to prevent conflicts. :)

@filipekiss
Copy link
Author

As the pull-request itself is 5 years old and the project hasn't received a commit in almost 3 years, I think it's safe to close this.

@filipekiss filipekiss closed this Feb 24, 2020
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

Successfully merging this pull request may close these issues.

3 participants