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

Set Path From Document isn't working properly #352

Closed
scriptum opened this issue Oct 10, 2014 · 10 comments

Comments

Projects
None yet
5 participants
@scriptum
Copy link
Member

commented Oct 10, 2014

After some time Set Path From Document stops to work. I looked through the code and sure that problem is somewhere in vte_get_working_directory with of pid = 0.

How to reproduce (not sure that will work for all configurations):

  1. Restart Terminal
  2. Restart Terminal again
  3. Try to set path from different documents (especially with non-ASCII filenames).

Maybe this happens only with Russian locale. If not reproducible, try to run under ru or some other locale.

How could this be fixed:

  1. Remove if (! utils_str_equal(path, vte_info.dir)) check. Not sure that this is very necessary.
  2. Check vte_get_working_directory logic. Sometimes pid variable becomes 0 and this causes a bug.
  3. Check vte_restart - why do we need to kill shell? Also shells support reset command.

VTE and "Set Path From Document" also has a bug when I use C locale with Russian file paths: cd '/home/rpg/?????????/?????? ?? GTK ? Cairo' and so on.

@codebrainz

This comment has been minimized.

Copy link
Member

commented Oct 10, 2014

On 14-10-10 02:04 PM, RPG wrote:

[snip] VTE and "Set Path From Document" also has a bug when I use C
locale with Russian file paths: cd '/home/rpg/?????????/?????? ?? GTK ? Cairo' and so on.

This is probably because GLib uses the locale encoding as the filename
encoding, so it would totally choke trying to convert C locale string
with Russian characters into UTF-8. If you're able to try it on Windows,
it would probably work OK as GLib is hard-coded to use UTF-8 for
filename encoding there.

@scriptum

This comment has been minimized.

Copy link
Member Author

commented Oct 12, 2014

This is probably because GLib uses the locale encoding as the filename encoding

Strange, I tried Terminator (also based on vte and GLib/GTK) with C locale but it worked properly with Russian characters.

BTW everything in Geany works ok with C locale except vte:

@b4n

This comment has been minimized.

Copy link
Member

commented Oct 12, 2014

Strange, I tried Terminator (also based on vte and GLib/GTK) with C locale but it worked properly with Russian characters.

MATE Terminal (which is basically GNOME 2 terminal) has the very same problem as Geany's VTE. This is because it uses the default character encoding from the system (through the LANG variable I guess), and then guesses ASCII (to be exact, ANSI_X3.4-1968 -- see g_get_charset()).
I don't know how Terminator does it, but if it forces UTF-8 it might be wrong in some systems -- although indeed most current Linux systems use UTF-8.

BTW everything in Geany works ok with C locale except vte:

Sure, because Geany doesn't use the system encoding for its interface, but like all GTK apps UTF-8. Also, I believe that if it works with locale filenames is only because our utils_get_utf8_from_locale() falls back to guessing the system is UTF-8 if the conversion fails. This is a bit crazy as there is no real reason why it would be the case, but in most case it is so no one ever complained.

@codebrainz

This comment has been minimized.

Copy link
Member

commented Oct 12, 2014

On 14-10-12 09:39 AM, RPG wrote:

This is probably because GLib uses the locale encoding as the filename encoding

Strange, I tried Terminator (also based on vte and GLib/GTK) with C locale but it worked properly with Russian characters.

One thing you might try is to set G_FILENAME_ENCODING environment
variable to the encoding that is used on that file system for file
names, when it differs from a specified locale. According to the docs,
that should fix it.

@scriptum

This comment has been minimized.

Copy link
Member Author

commented Oct 12, 2014

  • G_FILENAME_ENCODING=UTF-8 LC_ALL=C geany - has no effect
  • playing with LC_* inside Geany - same

So LC_ALL=C geany breaks filename encoding inside Geany's vte (but not in Geany itself). But I afraid that has nothing to do with bug described above (Set Path From Document) and wrong encoding could be another bug.

@codebrainz

This comment has been minimized.

Copy link
Member

commented Oct 13, 2014

On 14-10-12 12:03 PM, RPG wrote:

  • G_FILENAME_ENCODING=UTF-8 LC_ALL=C geany - has no effect
  • playing with LC_* inside Geany - same

So LC_ALL=C geany breaks filename encoding inside Geany's vte (but not in Geany itself).

Are the filenames actually encoded in UTF-8? Is that what your usual
locale encoding is (ie. locale charmap)? Maybe just try to set
G_BROKEN_FILENAMES env. var.

@elextr

This comment has been minimized.

Copy link
Member

commented Oct 13, 2014

LC_ALL=C officially supports only ASCII, so that can't support Russian characters. Clearly some apps have heuristics or assumptions/fallbacks that work (sometimes?) and some (eg VTE) don't.

The filesystem drivers should convert all filenames to the native system locale ("") but clearly that isn't affected when you set a local locale for just one process. Geany sets its locale to "" explicitly, so it should match the encoding provided by the system. VTE is probably using the current locale not the system locale and getting 'C' which doesn't support Russian characters so it gets confused.

@scriptum

This comment has been minimized.

Copy link
Member Author

commented Oct 13, 2014

Okay, I'll try to check it on virtual machine with default English locale. My current locale supports UTF-8: ru_RU.UTF-8.

@elextr

This comment has been minimized.

Copy link
Member

commented Oct 13, 2014

To correct myself, the conversions I referred to only apply to some (Windows) filesystems and only if mount options are set. They do not happen for extfs for example. In that case the filesystem is WYSIWYGB (what you set is what you get back :) so it depends on applications to agree on what encoding is to be used. I don't quite know how this is supposed to work if applications use the locale data since it can be changed by the user.

Bring on a UTF8 world :)

eht16 added a commit to eht16/geany that referenced this issue Mar 31, 2019

Restart the shell if VTE restart is requested
This happened in the past until d35e664
because of https://sourceforge.net/p/geany/bugs/163/ and
https://bugzilla.gnome.org/show_bug.cgi?id=540161.
But since the VTE bug has been fixed for long, we can remove the
workaround.

Closes geany#352.
@eht16

This comment has been minimized.

Copy link
Member

commented Mar 31, 2019

#2116 should fix the original issue (Set path from document broken after VTE restart).
The reason for pid=0 was the removed vte_start() call which we removed long time ago because of a VTE bug. But the VTE bug has been fixed also long time ago, so we should explicitly start the shell again and so get a valid pid again and everything else will work again.

@eht16 eht16 closed this in #2116 Apr 28, 2019

@b4n b4n added this to the 1.35 milestone Apr 28, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.