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

Removed $pwd from showing in Directory History listing in the CTRL+L tab(LOCATION) #527 #531

Merged
merged 4 commits into from Dec 16, 2017
Merged

Conversation

@adracea
Copy link
Contributor

@adracea adracea commented Dec 13, 2017

The make passed successfully with those changes and the behavior is the intended one . Awaiting comments and feedback as this is my first serious contribution to the github&open-source community .

edit/location.go Outdated
@@ -139,6 +139,20 @@ func (loc *location) Accept(i int, ed *Editor) {
ed.mode = &ed.insert
}

//This removes the $pwd from the current directory-historyDue to the way the paths are handled before this operation , we have to get all paths evaluated to the homedir , which gives
Copy link
Member

@xiaq xiaq Dec 13, 2017

Choose a reason for hiding this comment

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

You can just add pwd to the blacklist on line 164. There is no need to convert the directory values to absolute paths either; the directory entries in storage always contain results from os.Getwd (c.f. https://github.com/elves/elvish/blob/master/eval/chdir.go).

Loading

Copy link
Contributor Author

@adracea adracea Dec 13, 2017

Choose a reason for hiding this comment

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

I'll look into it tomorrow morning . Thanks for the warm welcome 😄 .

Loading

Copy link
Contributor Author

@adracea adracea Dec 14, 2017

Choose a reason for hiding this comment

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

Okay , I figured out why on earth my initial commit wasn't working as intended when I started trying it out at home . So : util.Getwd() -- the function that returns just the string of the pwd , returns it as it gets displayed : as "~/go/src/elves/elvish" but ; os.Getwd() returns it as "/home/user/go/src/elves/elvish" . I was trying to put util.Getwd() in the blacklist while the directory listing had os.Getwd() history in it .
In the current form , I would overwrite the current blacklist/pinned list .
@xiaq How would one blacklist a folder ?
I'll resubmit a full solution tomorrow morning .

Loading

@xiaq
Copy link
Member

@xiaq xiaq commented Dec 13, 2017

Welcome to the open source community! :)

Loading

edit/location.go Outdated
@@ -148,6 +148,8 @@ func locStart(ed *Editor) {
// Pinned directories are also blacklisted to prevent them from showing up
// twice.
black := convertListsToSet(ed.locHidden(), ed.locPinned())
pwd, _ := os.Getwd()
Copy link
Member

@xiaq xiaq Dec 15, 2017

Choose a reason for hiding this comment

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

Please handle the possible error.

If there was an error, you can simply not add pwd to the blacklist.

Loading

@xiaq xiaq merged commit 6c8cb5c into elves:master Dec 16, 2017
2 checks passed
Loading
@xiaq
Copy link
Member

@xiaq xiaq commented Dec 16, 2017

Thank you! :)

Loading

@notramo
Copy link

@notramo notramo commented Dec 16, 2017

Thanks.

Loading

@adracea
Copy link
Contributor Author

@adracea adracea commented Dec 16, 2017

Thanks for including it ! :)
Glad I could contribute to elvish since I've been using it for a while now both at home(+phone) and at work ! I'll keep contributing however I can around here . 😃

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants