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

Default color output for ls on OSX #3141

Merged
merged 1 commit into from Apr 13, 2013
Merged

Default color output for ls on OSX #3141

merged 1 commit into from Apr 13, 2013

Conversation

boxed
Copy link
Contributor

@boxed boxed commented Apr 6, 2013

Also fixed incorrect overriding of ls alias on OSX which prevented producing color output.

@boxed
Copy link
Contributor Author

boxed commented Apr 6, 2013

I'm a bit unsure of the change I made for avoiding aliases to get overwritten. It seems to me that there might be cases where you DO want aliases to be overwritten that my change will stop. I'm not sure how you should differentiate between these cases.

@minrk
Copy link
Member

minrk commented Apr 6, 2013

OS X does not behave differently from BSD here, so we don't need to add an extra branch, we just needed to add -Gto the BSD, OS X, etc. branch.

Fixed incorrect overriding of ls alias on OSX which prevented producing color output
@boxed
Copy link
Contributor Author

boxed commented Apr 8, 2013

Aha. Fixed now.

@minrk
Copy link
Member

minrk commented Apr 8, 2013

I can't imagine a case where an alias should be overwritten by a regular executable on the path. The point of aliases is to emulate bash aliases, which always take precedence over items on your path (as far as I know, anyway).

@boxed
Copy link
Contributor Author

boxed commented Apr 8, 2013

Well it happens on my machine... I tried changing the code like this:

                                if ff in self.shell.alias_manager.alias_table:
                                    print "'%s' overwrites %s" % (ff, self.shell.alias_manager.alias_table[ff])
                                self.shell.alias_manager.define_alias(
                                    ff.replace('.',''), ff)

..and it prints

'clear' overwrites (0, 'clear')
'less' overwrites (0, 'less')
'man' overwrites (0, 'man')
'more' overwrites (0, 'more')
'cat' overwrites (0, 'cat')
'cp' overwrites (0, 'cp -i')
'ls' overwrites (0, 'ls -G')
'mkdir' overwrites (0, 'mkdir')
'mv' overwrites (0, 'mv -i')
'rm' overwrites (0, 'rm -i')
'rmdir' overwrites (0, 'rmdir')
'easy_install' overwrites (0, 'easy_install')
'freetype-config' overwrites (0, 'freetype-config')
'libpng-config' overwrites (0, 'libpng-config')

@minrk
Copy link
Member

minrk commented Apr 8, 2013

Sorry, I meant that I can't imagine a case where that's the right behavior - you said you were unsure about your change preventing the code from overwriting aliases with items on the path, and I only meant to say that I can't think of a situation where your change is not the right answer. Basically, a circuitous and unclear +1 to the change.

@boxed
Copy link
Contributor Author

boxed commented Apr 9, 2013

Aha. Cool :)

On 8 apr 2013, at 22:33, Min RK notifications@github.com wrote:

Sorry, I meant that I can't imagine a case where that's the right behavior - you said you were unsure about your change preventing the code from overwriting aliases with items on the path, and I only meant to say that I can't think of a situation where your change is not the right answer. Basically, a circuitous and unclear +1 to the change.


Reply to this email directly or view it on GitHub.

@ellisonbg
Copy link
Member

Yeah colored ls. This looks good, I am merging.

ellisonbg added a commit that referenced this pull request Apr 13, 2013
Default color output for ls on OSX
@ellisonbg ellisonbg merged commit 90f9724 into ipython:master Apr 13, 2013
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
Default color output for ls on OSX
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.

None yet

3 participants