Skip to content

Support dired-subtree#13

Open
myuhe wants to merge 4 commits into
sabof:masterfrom
myuhe:subtree_support
Open

Support dired-subtree#13
myuhe wants to merge 4 commits into
sabof:masterfrom
myuhe:subtree_support

Conversation

@myuhe

@myuhe myuhe commented Feb 24, 2015

Copy link
Copy Markdown
Contributor

dired-subtree repositry is here.

@myuhe myuhe changed the title Suppot dired-subtree Support dired-subtree Feb 24, 2015
@myuhe

myuhe commented Aug 9, 2015

Copy link
Copy Markdown
Contributor Author

ping

Comment thread stripe-buffer.el Outdated

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Perhaps

(unless (and (fboundp 'dired-subtree--get-ov)
             (dired-subtree--get-ov))
  )

? It covers both cases, and I'd rather avoid loading a package.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes.
(require 'dired-subtree nil t) covers both case.

dired-subtree--get-ov may chage on upstream.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

AFAIK, require will try to find the package in the filesystem, if it's not loaded. Which is not an option in a loop.

You are using dired-subtree--get-ov anyway. Should upstream change fboundp is more likely to fail silently -- which I think is more desirable.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If dired-subtree is loaded, dired-subtree--get-ov is defined, and (featurep 'dired-subtree) returns t .
This branch never uses dired-subtree--get-ov as long as load dired-subtree

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

What you wrote is not equivalent to featurep. It's extremely slow if it the library is not installed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I understood what you say. Thanks.
I commit myuhe@aa139e2 that fix it.

See it.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

You are no longer checking if you are within a subtree

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If dired-subtree--get-ov is defined, dired-subtree should be loaded.
Is fboundp more desireble, isn't it?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Didn't you mean to write this?

(and (fboundp 'dired-subtree--get-ov)
             (dired-subtree--get-ov))

, with (dired-subtree--get-ov) testing whether inside a subtree?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks.
I fixed it again. myuhe@3a980dd
check it.

Comment thread stripe-buffer.el Outdated

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Now that I see it again, this looks convoluted -- you have an extra unneeded conditional. You probably want not instead of unless. Or even better, remove the negation, and reverse the clauses.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I messed it up... Thanks.
FIxed it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Please Check it again.

@sabof

sabof commented Aug 28, 2015

Copy link
Copy Markdown
Owner

Sorry, I'm being a crappy maintainer. I've played with it a bit. Is your goal basically to remove stripes from the subtrees? If yes, skipping is quite easy. It's likely also a better solution, since having subtle colour variations mean different things is likely to be more confusing than helpful for most people.

@myuhe

myuhe commented Aug 28, 2015

Copy link
Copy Markdown
Contributor Author

Why is "more confusing"?

If stripe-buffer overlays colourful stiripes per directory level, I think that it is easy to understand directory level .

Try to use subtree_support branch.

@sabof

sabof commented Aug 28, 2015

Copy link
Copy Markdown
Owner

That is the branch I've tested. Could you post a screenshot of your setup, showing different directories open?

@myuhe

myuhe commented Aug 28, 2015

Copy link
Copy Markdown
Contributor Author

Here it is.

_045

@myuhe

myuhe commented Sep 15, 2015

Copy link
Copy Markdown
Contributor Author

ping again.

@sabof

sabof commented Oct 4, 2015

Copy link
Copy Markdown
Owner

That's fine, there is a small bug, I'll merge your changes along with the fix.

@myuhe

myuhe commented Oct 5, 2015

Copy link
Copy Markdown
Contributor Author

Thanks!
I'm looking forward to merge.

@glucas

glucas commented Mar 17, 2016

Copy link
Copy Markdown

Any chance for merging this? Looks useful.

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