Add templates to Config Files menu #3396

ntrel · 2023-02-13T11:40:05Z

Also fix the first line of geany.css being uncommented when opening the config file through the menus.

Move config_files_menu to ui_widgets.

ntrel · 2023-04-09T14:34:27Z

I've split out #3450 which should be merged first.
After that, I'd like to merge this.

elextr · 2023-04-10T01:42:35Z

I guess it saves users explicitly copying template files to the user config directory before editing and maybe reduces the incidence of them editing the system copies. But most templates distributed with Geany are pretty ummm basic, so I'm not sure how often they are used as a basis for user templates.

Note the #3450 changes still included, needs to be removed, especially if #3450 is not needed.

src/templates.c

src/ui_utils.c

techee · 2023-10-06T16:54:23Z

Overall the PR looks good to me.

ntrel · 2023-10-06T17:04:35Z

@techee Thanks for reviewing. I've approved #3576.

kugel- · 2023-10-07T22:09:32Z

This touches strings (doesn't it?) and we're in string freeze so I'm afraid it's too late now.

ntrel · 2023-10-08T08:26:22Z

@kugel- what bad thing would happen if this was merged? the strings here would still lookup the correct translations, right?

elextr · 2023-10-08T09:31:01Z

Don't strings have file/line in their lookup, so "Templates" and "Files" menu items would likely show untranslated until updated?

Also I don't see those referenced in the documentation modifications?

eht16 · 2023-10-08T10:19:24Z

Don't strings have file/line in their lookup, so "Templates" and "Files" menu items would likely show untranslated until updated?

How do you think so? AFAIK the lookup is made solely on the string itself. Either it matches or not.

Before further speculating, just test it. Check out the branch, run make -C po update-po and check whether it broke any ".po" file.

Also I don't see those referenced in the documentation modifications?

I don't understand what does this mean? :).

elextr · 2023-10-08T10:53:59Z

How do you think so? AFAIK the lookup is made solely on the string itself. Either it matches or not.

Oh, I thought it was possible to have the same string at different places in the program with differing translations? If not by file/line how is that done? (translation newby question)

Did I say it would break anything? Just that the strings would be untranslated, and even if all cases of the same string have the same translations, do "Templates" or "Files" occur in the UI anywhere already?

I'm not sure what the point of running update PO is, we don't do that for each PR. That was done at string freeze and will not have these strings in it, so they will not be translated thats all.

I don't understand what does this mean? :).

There are changes to geany.txt included in this PR, so I expected they were the documentation associated with this change. But weirdly the documentation changes do not use either of the words Templates or Files so I'm not sure how they document the new menu entries ;-)

eht16 · 2023-10-08T12:14:27Z

doc/geany.txt

@@ -3973,8 +3973,8 @@ On Windows 7 and above you most likely will find it at:
 ``C:\users\UserName\Roaming\geany``


-Tools menu items
----------------
+Configuration files menu


While this section is currently mainly about the Configuration files menu item, I think we could leave the section title as is. We might add further documentation for other Tools menu items here as well, so the more generic section title would serve as a container and so we could add a sub heading for Configuration files.

Though I don't have a strong opinion on that.

eht16 · 2023-10-08T12:25:43Z

How do you think so? AFAIK the lookup is made solely on the string itself. Either it matches or not.

Oh, I thought it was possible to have the same string at different places in the program with differing translations? If not by file/line how is that done? (translation newby question)

AFAIK this is not possible. If so, then there is some special syntax or so. Usually, the lookup is done based only on the string.
Example for the string "Templates" used in here:

#: data/geany.glade:5527 src/prefs.c:1625 src/templates.c:325
msgid "Templates"
msgstr ""

As you can see the translation item has the multiple code occurrences listed as comment.

Maybe @frlan knows better.

Did I say it would break anything? Just that the strings would be untranslated, and even if all cases of the same string have the same translations[...]

I meant "break" in terms of "new untranslated strings which would break the string freeze". Sorry, it wasn't the best wording.

do "Templates" or "Files" occur in the UI anywhere already?

Yes, see the example above.

I'm not sure what the point of running update PO is, we don't do that for each PR.

I suggested this so one can test whether the added strings here will be translated or not to stop us speculating whether or not it will have an effect.
To get this further, I just did it myself: applied the patch, ran make -C po update-po, checked the re-generated translation files, there are no new untranslated strings, ran Geany with a non-English locale and saw that the new menu items got properly translated by the already existing translations for "Template" and "Files".

That was done at string freeze and will not have these strings in it, so they will not be translated thats all.

This isn't the case, see above.
I wonder why we discuss this so long with much guessing what might happen instead of just testing it to get sure :(.

I don't understand what does this mean? :).

There are changes to geany.txt included in this PR, so I expected they were the documentation associated with this change. But weirdly the documentation changes do not use either of the words Templates or Files so I'm not sure how they document the new menu entries ;-)

To me the changed parts in the documentation are related to the feature.

eht16 · 2023-10-08T12:26:17Z

Long story short about the translation strings: this is safe to merge, it will NOT add new untranslated strings.

elextr · 2023-10-08T12:43:33Z

As you can see the translation item has the multiple code occurrences listed as comment.

After some googling (while also watching renovation TV so it was a bit slow :-) I found that:

a string can only occur once in the PO files, so one string and commenting the places its used is the only way
if a string needs two translations, like as a noun and a verb, a manual msgctxt is needed to separate them, not file/line.

That is different to the commercial system I have used in the past. As I said I don't know much about gettext but I have learned some more. "learn a new thing every day" they say, so I can take the rest of the day off ... all 1.25 hours.

I wonder why we discuss this so long with much guessing what might happen instead of just testing it to get sure :(.

Because I am not at home sitting in front of my development machine to be able to do it.

Long story short about the translation strings: this is safe to merge, it will NOT add new untranslated strings.

Ok, so is the actual change ok? If so you and @techee are listed as reviewers and can approve it.

eht16 · 2023-10-11T10:42:42Z

Tested and works fine.

Maybe we could squash some of the commits?

eht16 · 2023-10-14T08:51:34Z

@ntrel @elextr

I tested and it works great.

Apart from the little remark on the docs, I'm fine with this.
About the section comment in the docs, this is no big deal, we can also leave as it is.

b4n

Looks fairly useful and simple enough 👍
Apart from my inline comments, LGTM and WFM.

b4n · 2023-10-16T22:09:31Z

src/templates.c

+	gtk_container_add(GTK_CONTAINER(menu), item);
+	gtk_menu_reorder_child(GTK_MENU(menu), item, 0);


Suggested change

gtk_container_add(GTK_CONTAINER(menu), item);

gtk_menu_reorder_child(GTK_MENU(menu), item, 0);

gtk_menu_shell_prepend(GTK_MENU_SHELL(menu), item);

b4n · 2023-10-16T22:25:26Z

src/ui_utils.c

-	/* put entries with submenus at the end of the menu */
+	/* put entries with submenus at the start of the menu */
 	if (gtk_menu_item_get_submenu(item_a) && !gtk_menu_item_get_submenu(item_b))
-		return 1;
-	else if (!gtk_menu_item_get_submenu(item_a) && gtk_menu_item_get_submenu(item_b))
 		return -1;
+	else if (!gtk_menu_item_get_submenu(item_a) && gtk_menu_item_get_submenu(item_b))
+		return 1;


This confused me because it's already in #3397, so it's actually not part of this PR anymore. But 👍 anyway.

b4n · 2023-10-17T21:23:06Z

Should we postpone? Review the comments and merge?

b4n · 2023-10-18T22:32:02Z

Postponing as it's a bit late now. Should probably be merged soon after release though.

ntrel added 2 commits February 13, 2023 11:31

Add new file templates to Config Files menu

e2ef69d

Move config_files_menu to ui_widgets.

List general templates too

52fce9d

ntrel added this to the 1.39/2.0 milestone Feb 15, 2023

ntrel mentioned this pull request Apr 9, 2023

[config files] Don't comment global content unless it is Conf #3450

Closed

techee reviewed Oct 6, 2023

View reviewed changes

src/templates.c Show resolved Hide resolved

techee reviewed Oct 6, 2023

View reviewed changes

src/templates.c Show resolved Hide resolved

techee reviewed Oct 6, 2023

View reviewed changes

src/ui_utils.c Outdated Show resolved Hide resolved

ntrel added 4 commits October 6, 2023 16:07

Put items with submenus at the top of the menu

937485a

Update docs

558c70a

Tweak template wildcards formatting

4da55f9

Move Files submenu to top

abfaf3b

ntrel force-pushed the conf-file-templates branch from f5a7aa5 to abfaf3b Compare October 6, 2023 15:14

eht16 reviewed Oct 8, 2023

View reviewed changes

eht16 approved these changes Oct 11, 2023

View reviewed changes

b4n reviewed Oct 16, 2023

View reviewed changes

b4n modified the milestones: 2.0, 2.1 (or something?) Oct 18, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add templates to Config Files menu #3396

Add templates to Config Files menu #3396

ntrel commented Feb 13, 2023

ntrel commented Apr 9, 2023

elextr commented Apr 10, 2023

techee commented Oct 6, 2023

ntrel commented Oct 6, 2023

kugel- commented Oct 7, 2023

ntrel commented Oct 8, 2023

elextr commented Oct 8, 2023

eht16 commented Oct 8, 2023

elextr commented Oct 8, 2023

eht16 Oct 8, 2023

eht16 commented Oct 8, 2023

eht16 commented Oct 8, 2023

elextr commented Oct 8, 2023

eht16 commented Oct 11, 2023

eht16 commented Oct 14, 2023

b4n left a comment

b4n Oct 16, 2023

b4n Oct 16, 2023

b4n commented Oct 17, 2023

b4n commented Oct 18, 2023

		gtk_container_add(GTK_CONTAINER(menu), item);
		gtk_menu_reorder_child(GTK_MENU(menu), item, 0);

	gtk_container_add(GTK_CONTAINER(menu), item);
	gtk_menu_reorder_child(GTK_MENU(menu), item, 0);
	gtk_menu_shell_prepend(GTK_MENU_SHELL(menu), item);

Add templates to Config Files menu #3396

Are you sure you want to change the base?

Add templates to Config Files menu #3396

Conversation

ntrel commented Feb 13, 2023

ntrel commented Apr 9, 2023

elextr commented Apr 10, 2023

techee commented Oct 6, 2023

ntrel commented Oct 6, 2023

kugel- commented Oct 7, 2023

ntrel commented Oct 8, 2023

elextr commented Oct 8, 2023

eht16 commented Oct 8, 2023

elextr commented Oct 8, 2023

eht16 Oct 8, 2023

Choose a reason for hiding this comment

eht16 commented Oct 8, 2023

eht16 commented Oct 8, 2023

elextr commented Oct 8, 2023

eht16 commented Oct 11, 2023

eht16 commented Oct 14, 2023

b4n left a comment

Choose a reason for hiding this comment

b4n Oct 16, 2023

Choose a reason for hiding this comment

b4n Oct 16, 2023

Choose a reason for hiding this comment

b4n commented Oct 17, 2023

b4n commented Oct 18, 2023