RSS/Atom feed Twitter
Site is read-only, email is disabled

A few simple patches

This discussion is connected to the gimp-developer-list.gnome.org mailing list which is provided by the GIMP developers and not related to gimpusers.com.

This is a read-only list on gimpusers.com so this discussion thread is read-only, too.

6 of 6 messages available
Toggle history

Please log in to manage your subscriptions.

A few simple patches David Gowers 26 Jun 04:28
  A few simple patches Sven Neumann 26 Jun 08:50
   A few simple patches gg@catking.net 26 Jun 12:50
    A few simple patches Sven Neumann 26 Jun 19:09
     A few simple patches David Gowers 27 Jun 03:31
      A few simple patches Sven Neumann 27 Jun 08:33
David Gowers
2007-06-26 04:28:15 UTC (almost 17 years ago)

A few simple patches

filepath.diff adds a 'file path' field to the 'Image Properties' dialog; I did this after some discussion between gg and myself on the importance of being able to find out the path easily. Image_metadata.diff changes the title used by the 'metadata' plugin for its window. Mainly, it's just so that there are not two different dialogs both titled 'Image Properties' available. Maybe the title should be 'File Properties' instead -- but, IMV, this is ambiguous VS 'Image properties' -- 'metadata' is the more correct term.

Neither of these patches seem controversial (although they do effect translations), which is why I posted them here instead of on Bugzilla. I hope that the ML software doesn't strip off these attachments :)

Sven Neumann
2007-06-26 08:50:44 UTC (almost 17 years ago)

A few simple patches

Hi,

On Tue, 2007-06-26 at 11:58 +0930, David Gowers wrote:

filepath.diff adds a 'file path' field to the 'Image Properties' dialog; I did this after some discussion between gg and myself on the importance of being able to find out the path easily.

That's already on my TODO but I will probably do it differently than your approach.

Image_metadata.diff changes the title used by the 'metadata' plugin for its window. Mainly, it's just so that there are not two different dialogs both titled 'Image Properties' available.

The metadata plug-in is unfinished and not supposed to be part of the next stable release anyway. Perhaps it is time that we stop building and installing it.

Sven

gg@catking.net
2007-06-26 12:50:45 UTC (almost 17 years ago)

A few simple patches

On Tue, 26 Jun 2007 08:50:44 +0200, Sven Neumann wrote:

Hi,

On Tue, 2007-06-26 at 11:58 +0930, David Gowers wrote:

filepath.diff adds a 'file path' field to the 'Image Properties' dialog; I did this after some discussion between gg and myself on the importance of being able to find out the path easily.

That's already on my TODO but I will probably do it differently than your approach.

Unless what he's implemented is bad why not just comit anyway until you get around to doing it better/diffeently? It would be one less task on your busy schedule and one more lose end tidied up for 2.4

Image_metadata.diff changes the title used by the 'metadata' plugin for its window. Mainly, it's just so that there are not two different dialogs both titled 'Image Properties' available.

The metadata plug-in is unfinished and not supposed to be part of the next stable release anyway. Perhaps it is time that we stop building and installing it.

Well it's true it does not seem to do much so it may be best no to build it if it's been pushed to a future release. Still committing the name change would not be a bad move since David has coded it.

You've said you were very frustrated about not getting 2.4 out yet. May I gently make the point that if you could be a little more flexible in accepting others contributions even if not _exactly_ the way you would chose for it to be done you would get more contributions and contributers.

Maybe you dont realise how frustrating and demoralising it is to put in time to come up with improvements, code and submit patchs only to see them bounced.

It really leaves one with the feeling "why did I bother?".

Clearly you're not going to accept everything anyone comes up with but everyone needs to find thier motivation for contributing to OSS. I think this often means fixing an personal annoyance or adding a feature we'd like to see. Getting things refused without a what seems to be a firm reason is disheartening and removes the motivation to make further contributions.

Please don't take that the wrong way , it's just something you may like to consider that could help the project gain momentum.

regards, gg/

Sven Neumann
2007-06-26 19:09:09 UTC (almost 17 years ago)

A few simple patches

Hi,

On Tue, 2007-06-26 at 12:50 +0200, gg@catking.net wrote:

Unless what he's implemented is bad why not just comit anyway until you get around to doing it better/diffeently?

I rejected the patch for several reasons, all of them technical:

(1) It adds a label for a potentially long string without taking any measure to avoid that the dialog gets too wide due to that.

(2) It mixes filenames with strings displayed in the GUI. Filenames can be of a different encoding and therefore need special treatment. In particular you must not call g_path_get_dirname() on the result of file_utils_uri_display_name().

(3) Showing a directory name does only work for local files, it breaks for remote files.

Well it's true it does not seem to do much so it may be best no to build it if it's been pushed to a future release. Still committing the name change would not be a bad move since David has coded it.

Committing that change would have introduced a string change. We are tentatively string frozen so we will avoid any string changes that are not absolutely needed.

The Image Properties dialog only exists because the metadata plug-in is not yet ready for prime time. As soon as the plug-in is considered complete and stable, we won't need the core dialog any longer and the two dialogs can be merged into a single one.

Maybe you dont realise how frustrating and demoralising it is to put in time to come up with improvements, code and submit patchs only to see them bounced.

Sorry, but I couldn't accept the patches for the reasons given above. And now I have even put more time into this than it has taken David to come up with the patches in the first place.

As a general rule, please ask before you write a patch.

Sven

David Gowers
2007-06-27 03:31:57 UTC (almost 17 years ago)

A few simple patches

On 6/27/07, Sven Neumann wrote:

Hi,

On Tue, 2007-06-26 at 12:50 +0200, gg@catking.net wrote:

Unless what he's implemented is bad why not just comit anyway until you get around to doing it better/diffeently?

I rejected the patch for several reasons, all of them technical:

(1) It adds a label for a potentially long string without taking any measure to avoid that the dialog gets too wide due to that.

Okay (I don't know how to ellipsize a string in GTK, though of course I know how to truncate or ellipsize the content of it)

(2) It mixes filenames with strings displayed in the GUI. Filenames can be of a different encoding and therefore need special treatment. In particular you must not call g_path_get_dirname() on the result of file_utils_uri_display_name().

So the filename should be recoded to ascii and then finally back to UTF-8? (IIRC URI's are encoded in UTF-8.)

(3) Showing a directory name does only work for local files, it breaks for remote files.

This is incorrect, I checked for that case and it shows what I believe to be the correct parallel to 'directory name': for 'http://foo.bar/baz/bif.gif' it shows 'http://foo.bar/baz/' (the only other parallel I could see making sense would be /baz/ -- which strikes me as not-very-helpful.)

Sorry, but I couldn't accept the patches for the reasons given above. And now I have even put more time into this than it has taken David to come up with the patches in the first place.

As a general rule, please ask before you write a patch.

Okay, I will.

Sven Neumann
2007-06-27 08:33:08 UTC (almost 17 years ago)

A few simple patches

Hi,

On Wed, 2007-06-27 at 11:01 +0930, David Gowers wrote:

(2) It mixes filenames with strings displayed in the GUI. Filenames can be of a different encoding and therefore need special treatment. In particular you must not call g_path_get_dirname() on the result of file_utils_uri_display_name().

So the filename should be recoded to ascii and then finally back to UTF-8? (IIRC URI's are encoded in UTF-8.)

No, filenames are not in ASCII encoding. GLib has functions to convert from and to filename encoding but it would go beyond the scope of this mailing-list to explain this here.

Since I have already taken care of the issue in SVN, I don't think we need to discuss this further.

(3) Showing a directory name does only work for local files, it breaks for remote files.

This is incorrect, I checked for that case and it shows what I believe to be the correct parallel to 'directory name': for 'http://foo.bar/baz/bif.gif' it shows 'http://foo.bar/baz/' (the only other parallel I could see making sense would be /baz/ -- which strikes me as not-very-helpful.)

It would have broken on Windows where the path separator for filenames is different from the path separator in URIs.

Sven