p-e-w / finalterm

At last – a modern terminal emulator (NO LONGER MAINTAINED)
http://finalterm.org
GNU General Public License v3.0
3.84k stars 179 forks source link

Fix #338 #361

Closed nashley closed 9 years ago

nashley commented 9 years ago

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

p-e-w commented 9 years ago

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

cuttlebit commented 9 years ago

Do i fork a new copy and pull again?

nashley commented 9 years ago

@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 commented 9 years ago

Oh ok cool :)

nashley commented 9 years ago

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 commented 9 years ago

hmmm, still works for me. Strange.

2_001

nashley commented 9 years ago

@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 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

@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 commented 9 years ago

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 commented 9 years ago

@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 commented 9 years ago

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

nashley commented 9 years ago

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

cuttlebit commented 9 years ago

@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 commented 9 years ago

@Carnassial Sorry for doubting you again :sweat_smile: 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 commented 9 years ago

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

p-e-w commented 9 years ago

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

Merged! Credit to @Carnassial for the fix.