Skip to content
This repository has been archived by the owner on Nov 11, 2018. It is now read-only.

Fix #338 #361

Merged
merged 1 commit into from
Oct 14, 2014
Merged

Fix #338 #361

merged 1 commit into from
Oct 14, 2014

Conversation

nashley
Copy link
Contributor

@nashley nashley commented Sep 14, 2014

Fixes #338 by including hidden files in ls -a output. Thanks to Carnassial for this fix.

@p-e-w
Copy link
Owner

p-e-w commented Sep 23, 2014

Looks great, but three commits for one changed line... could you squash them into one, please? 😃

@cuttlebit
Copy link
Contributor

Do i fork a new copy and pull again?

@nashley
Copy link
Contributor Author

nashley commented Sep 25, 2014

@p-e-w It should be good to go now.
@carnassial I squashed three commits down to a single commit. You can read about how this is done here.
Essentially, I ran the following commands:
$ git stash to stash my local, uncommitted changes so that I can remove them for now but save them to work on later.
$ git checkout master to select the master branch, where this bug was fixed (usually, you should use a separate branch; that was a mistake on my part)
$ git rebase --interactive HEAD~2 to rebase the last commits (the commands should be self-explanatory; you basically just pick which commits to keep and which to merge with previous commits by editing a file)
$ git push -f this should usually be avoided as it will mess up the commit history (thus a separate branch is normally used), but it's alright since no one forked my fork after these commits were made. Essentially, it will forcibly remove the three commits and replace them with this one, squashed commit.

@cuttlebit
Copy link
Contributor

Oh ok cool :)

@nashley
Copy link
Contributor Author

nashley commented Sep 25, 2014

Unfortunately, I just tested this, and I was able to reproduce the bug again—even with this change applied. Could someone test this out to make sure that I'm not crazy? I remember this fix working previously, but it does not seem to be working for me anymore.

Screenshots below:
screenshot from 2014-09-25 04 57 33
screenshot from 2014-09-25 04 57 18

@cuttlebit
Copy link
Contributor

hmmm, still works for me. Strange.

2_001

'*' expands to itself if no expansions could be made, overriding every other file in ls_output.
Hidden files also have semantic menus now.
@nashley
Copy link
Contributor Author

nashley commented Sep 25, 2014

@carnassial Could you clone this again into a separate, temporary directory and build it again? I just cloned it again, and I can reproduce the bug.

@gmittert
Copy link

I cloned Vreality's fork into another directory and can reproduce the bug. I'm not sure if it's related, but I've got some interesting behaviour with aliasing ls, which might explain Carnassial's results.

snap

@cuttlebit
Copy link
Contributor

Mmmmm I cloned again, and I can reproduce the bug as well. Weird. I'll investigate.

When you "ls -a" and see a '*' being displayed, that means * is expanding to itself.

@jmittert, after you aliased, the directories are being displayed horizontally, which means that it's not using finalterm's ls.

UPDATE:
So apparently the fix works on my laptop, but not my desktop PC, don't know why yet.

@nashley
Copy link
Contributor Author

nashley commented Sep 25, 2014

@carnassial do you use bash on both your laptop and desktop? I'm not sure if it's an issue, but it could be (see the comment by @lwandrebeck on #26).

@cuttlebit
Copy link
Contributor

I use zsh on both my computers. Finalterm is in bash though.

Separate from finalterm;
It seems that zsh does not have shopt and matching fails with an error message if nothing is found rather than expanding to itself. Which is the same as if we used failglob in bash.

@nashley
Copy link
Contributor Author

nashley commented Sep 25, 2014

@carnassial I went back to your fork and checked out what I had previously though to be a working commit, but I sadly can't reproduce the fix. I thought that I had checked it myself, but I'm now not sure if I did; I wonder what's different on your laptop.

@cuttlebit
Copy link
Contributor

@VREALITY @jmittert
Guys, did we all forget to "sudo make install"?
I did that and it works on both my computers now. lol

@nashley
Copy link
Contributor Author

nashley commented Sep 26, 2014

@carnassial did you run ./finalterm (as opposed to just finalterm) beforehand? I don't think that installing it should make a difference.

@cuttlebit
Copy link
Contributor

@VREALITY I always use "./finalterm"

Installing apparently looks at Termlets, try it and see if it works :P.

-- Installing: /usr/local/share/finalterm/Termlets
-- Up-to-date: /usr/local/share/finalterm/Termlets/ls
-- Up-to-date: /usr/local/share/finalterm/Termlets/wget
-- Up-to-date: /usr/local/share/finalterm/Termlets/ps
-- Installing: /usr/local/share/finalterm/TextMenus

@nashley
Copy link
Contributor Author

nashley commented Sep 26, 2014

@carnassial Sorry for doubting you again 😅
That definitely fixes it, and I think it'd be safe to merge the commit now.
This brings up another topic though: is relying on system files for basic things such as termlets a good idea? It makes sense for binary packages, but not necessarily for portable solutions or when the program is compiled from source but not actually installed on the system.

@cuttlebit
Copy link
Contributor

The way Termlets work will probably need to be overhauled for zsh support anyway :)

@p-e-w p-e-w merged commit e623f0b into p-e-w:master Oct 14, 2014
@p-e-w
Copy link
Owner

p-e-w commented Oct 14, 2014

I can definitely confirm that this fixes the problem on my systems.

Merged! Credit to @carnassial for the fix.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hidden files not displaying with ls -a
4 participants