Re: [Geany-devel] Remove MSYS dependency of Geany on Win~1
On 08/07/2012 13:04, Nick Treleaven wrote: BTW I've run into 2 problems with the install target: 1. cp -r and xcopy treat the destination path differently. Currently MSYS install is broken and will install data files as $DESTDIR/data/data/* - I've replaced xcopy with separate copy targets locally. Unfortunately that causes errors with subdirectories so I had to ignore them, which I'd prefer not to do. See attached diff. Now fixed using ifdef MSYS. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Dropping Waf support?
On 16/07/2012 18:36, Enrico Tröger wrote: the make/MSYS build system support seems to get better and better due to Nick's and Dimitar's work on it Well, it's quite basic and needs a bit more work like allowing overridden CFLAGS. It doesn't install files like README, geany.html, etc. It also doesn't regenerate dependencies for each source file on-the-fly like autotools (and I expect Waf) does. It wouldn't bother me if Waf support was removed, but OTOH it looks like quite a lot of work went into it. I don't mind adding obvious updates to wscript when necessary, but can't promise to test them ;-) Nick ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Remove MSYS dependency of Geany on Win~1
On 09/07/2012 19:13, Dimitar Zhekov wrote: On Sun, 08 Jul 2012 13:04:54 +0100 Nick Treleavennick.trelea...@btinternet.com wrote: BTW I've run into 2 problems with the install target: 1. cp -r and xcopy treat the destination path differently. Currently MSYS install is broken and will install data files as $DESTDIR/data/data/* - I've replaced xcopy with separate copy targets locally. Unfortunately that causes errors with subdirectories so I had to ignore them, which I'd prefer not to do. See attached diff. cp: omitting directory 'foo' errors on any subdirectories with GNU cp. Wait... the _only_ 'cp' we have under Win~1 GNU cp, right? Instead of the patch, we can define CP_R as 'cp -r --remove-destination'. This non-POSIX option exists since at least fileutils-4.1 from 2001. Well, it'll remove $(DESTDIR)/data before copying, which I would prefer not to do, but there shoudn't be any user files anyway. No, the subdir errors are with windows 'copy'. It's not good to remove *all* destination files, it is valid to add files to the system config directory, understanding that they may get overwritten on upgrade /if/ a file with the same name is installed. This allows users to share some of the same config files. But this problem can be resolved either with separate copy commands as I attached previously, or maybe by having different install-data targets depending on whether MSYS is defined. 2. A weird problem. Installing from cmd.exe doesn't overwrite geany.glade (and maybe some other files) even though it appears to succeed. I only noticed this because I'm missing the View-Editor-Color Schemes menu which was added about a month ago (I tend to test in place rather than install each time). OMG... Then we must choose between copy and xcopy depending on which one works. Or we can define: Neither one works here ;-) But maybe it's something weird about my system or because I previously installed using MSYS cp. for cmd: 'CP_R = xcopy /s /y /i' and 'RM_R = del /s' xcopy /i doesn't seem to help here if the destination already exists (I don't think we should remove $DESTDIR). for MSYS: 'CP_R = cp -r' and 'RM_R = rm -r' and replace the '-$(MKDIR) $(DESTDIR)/data' with '-$(RM_R) $(DESTDIR)/data'. That will (a) nullify the differences between 'cp -r' and 'xcopy /s', (b) be a bit shorter than the individual MKDIR/CP commands from the patch, and (c) fix the overriding problem, since the target files will not exist - or maybe reveal something about the reason why it fails. Well, we haven't run out of options yet. I'll test the patch tomorrow. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Remove MSYS dependency of Geany on Win~1
On 07/07/2012 13:16, Dimitar Zhekov wrote: On Wed, 04 Jul 2012 12:59:11 +0100 Nick Treleavennick.trelea...@btinternet.com wrote: On 28/06/2012 18:55, Dimitar Zhekov wrote: BTW if you think plugins$(DIRSEP)*.dll looks ugly, we can use: plugins$/*.dll Nice. :) Though what I find a bit unpleasant is using the specific separator for a single install command, not the particular syntax. It may turn out to be needed elsewhere. This is what I intend to do. Then why not put it for all install commands? We'll not depend on the (undocumented AFAIK) behaviour of win~1 mkdir/copy/del to treat slash as a directory separator in some cases, but not in others. OK. BTW I've run into 2 problems with the install target: 1. cp -r and xcopy treat the destination path differently. Currently MSYS install is broken and will install data files as $DESTDIR/data/data/* - I've replaced xcopy with separate copy targets locally. Unfortunately that causes errors with subdirectories so I had to ignore them, which I'd prefer not to do. See attached diff. 2. A weird problem. Installing from cmd.exe doesn't overwrite geany.glade (and maybe some other files) even though it appears to succeed. I only noticed this because I'm missing the View-Editor-Color Schemes menu which was added about a month ago (I tend to test in place rather than install each time). Regards, Nick diff --git a/makefile.win32 b/makefile.win32 index 2f39eb4..fce209f 100644 --- a/makefile.win32 +++ b/makefile.win32 @@ -15,8 +15,7 @@ WINDRES = windres.exe CC = gcc CXX = g++ -CP = copy /Y -CP_R = xcopy /S /Y +CP = copy /y RM = del MKDIR = mkdir # $/ is used as a portable directory separator @@ -27,7 +26,6 @@ DESTDIR = C:/Program Files/Geany ifdef MSYS CP = cp -CP_R = cp -r RM = rm -f / = / endif @@ -62,7 +60,9 @@ clean: deps # likely requires admin privileges # mkdir output is ignored in case dir exists +# cp ignored because of subdirectories # 'copy' seems to only accept / in the destination +# xcopy has different destination semantics from cp -r, so we can't use it install: -$(MKDIR) $(DESTDIR) -$(MKDIR) $(DESTDIR)/bin @@ -70,4 +70,10 @@ install: -$(MKDIR) $(DESTDIR)/lib $(CP) plugins$/*.dll $(DESTDIR)/lib -$(MKDIR) $(DESTDIR)/data - $(CP_R) data $(DESTDIR)/data + -$(CP) data$/* $(DESTDIR)/data + -$(MKDIR) $(DESTDIR)/data/colorschemes + $(CP) data$/colorschemes$/* $(DESTDIR)/data/colorschemes + -$(MKDIR) $(DESTDIR)/data/templates + -$(CP) data$/templates$/* $(DESTDIR)/data/templates + -$(MKDIR) $(DESTDIR)/data/templates/files + $(CP) data$/templates$/files$/* $(DESTDIR)/data/templates/files ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Any suggestions where message is from?
On 07/07/2012 19:01, Colomban Wendling wrote: Compile with `-DG_FATAL_WARNINGS` and then run in gdb. It'll abort and you can see where it happened. …or run in GDB with G_DEBUG=fatal-warnings envvar set. Personally I use gdb with: r --g-fatal-warnings bt Nick ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Remove MSYS dependency of Geany on Win~1
On 28/06/2012 18:55, Dimitar Zhekov wrote: On Thu, 28 Jun 2012 14:41:25 +0100 Nick Treleavennick.trelea...@btinternet.com wrote: Yes, using a target for plugins only would be OK. What if we put the an install: target in plugins/makefile.win32? That works, needs no DIRSEP, and is very similar to the *nix installation. To avoid 'C:/Program Files/Geany already exists' on the first install, we will have to use mkdir -p, but that's the default behaviour of the cmd/tcc mkdir anyway. Attached, and the patch is git-ified this time. Well that removes DIRSEP, but adds MKDIR_P and adds defines to the plugins makefile. BTW if you think plugins$(DIRSEP)*.dll looks ugly, we can use: plugins$/*.dll This is what I intend to do. BTW (not included in the .diff): 'CP = copy' in doc/Makefile.win32 should be 'copy /Y'. OK, I'll fix it. 'MAKE =' seems unneeded, any GNU make defines is automatically. I already made that change last week. But I'm thinking of adding a MSYS variable now to make it easier: -include localwin32.mk ifdef MSYS CP = cp CP_R = cp -r RM = rm DIRSEP = / endif I thought of providing a sample localwin32.msys with the standard MSYS definitions. Then the user would have to keep merging it with their local one if we change/add definitions. In all cases, -include localwin32.mk should be the last, to allow overriding. Yes. And then perhaps use $(MAKE) CP=$(CP) RM=$(RM) to pass these variables to the sub-makefiles so they don't need to have copies of the ifdef. They only need CP and RM. Then all clean: targets will fail when run from a subdirectory (and the above install: when run from plugins/). So 'make all' in src/ will work, but not 'make clean'. Hmmm... Yes, I realized after sending the email. That means I will need to use 'ifdef MSYS' in any sub-makefiles that need it. This is the cleanest way to provide MSYS support. The user can even type MSYS=1 on the command-line quite easily. Also, I think you /can/ join command lines with a trailing backslash (but your patch change is still better to have STLIBS anyway): http://www.gnu.org/software/make/manual/html_node/Splitting-Lines.html It failed on my mingw32-make, I wound't have bothered if it worked. It seemed to work for me, and the manual seems to say that GNU make interprets the trailing backslash, it doesn't pass it to the shell. I have GNU Make 3.82. YMMV. (GNU)mingw32-make-3.82-5. Obviously treats \ different, since it's not an escape in C:\foo\bar. Anyway, STLIBS is a safe bet. Did you read the link? I'm talking about a backslash as the last character on the line. Nick ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Remove MSYS dependency of Geany on Win~1
On 27/06/2012 21:32, Dimitar Zhekov wrote: On Tue, 26 Jun 2012 21:21:57 +0100 Nick Treleavennick.trelea...@btinternet.com wrote: I've now committed to master an install target similar to your first patch, but using xcopy for data. I used a DIRSEP variable: $(CP) plugins$(DIRSEP)*.dll $(DESTDIR)/lib A make -C install-plugins will work without DIRSEP. Not ellegant, I'll be the first to admit - but supporting a directory separator variable specifically for a single command, when we can easily do without it, doesn't seem very pretty to me either. Yes, using a target for plugins only would be OK. But I'm thinking of adding a MSYS variable now to make it easier: -include localwin32.mk ifdef MSYS CP = cp CP_R = cp -r RM = rm DIRSEP = / endif And then perhaps use $(MAKE) CP=$(CP) RM=$(RM) to pass these variables to the sub-makefiles so they don't need to have copies of the ifdef. They only need CP and RM. Please test. I will. While unlikely, I'll check if 'xcopy /y plugins/*.dll ...' works. I tried that here and it failed. I think because 'copy' takes /y switches the first path it sees must not use forward slashes. They are OK in the destination though. Or maybe the forward slashes work with copy if the argument matches an existing path exactly, without find first/next file involved. Not that knowing the reason will help us. No, even full paths with forward slash do not work (at least for the first of the file arguments). Note that using a cmd-style shell with SHELL defined to a *nix style shell fails, no matter if makefile.win32 is patched or not. That's a problem of GNU make under Win~1 trying to be too smart, and ending up with things like CreateProcess (copy, ...) error 2. :) I don't get the last part above, but I think this is by design: The program used as the shell is taken from the variable SHELL. If this variable is not set in your makefile, the program /bin/sh is used as the shell. [...] But it doesn't really run the SHELL, just attempts to CreateProcess (copy, ...) because a SHELL is defined. Anyway, I only wanted to make it clear than in such (rare) configuration, mingw32-make is unlikely to work at all: any cmd command (mkdir, del) will be started directly as executable, and of course fail. xcopy should work though. It is weird that (without overriding on the command line) SHELL always is sh.exe, even when cmd.exe is obviously being used. Maybe a MinGW issue. It is as described in the manual, but why use cmd.exe then... Also, I think you /can/ join command lines with a trailing backslash (but your patch change is still better to have STLIBS anyway): http://www.gnu.org/software/make/manual/html_node/Splitting-Lines.html It failed on my mingw32-make, I wound't have bothered if it worked. It seemed to work for me, and the manual seems to say that GNU make interprets the trailing backslash, it doesn't pass it to the shell. I have GNU Make 3.82. YMMV. Regards, Nick ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Remove MSYS dependency of Geany on Win~1
On 26/06/2012 21:46, Nick Treleaven wrote: On 26/06/2012 21:31, Nick Treleaven wrote: On 26/06/2012 21:21, Nick Treleaven wrote: MSYS does treat backslashes as escapes, and I think that is a worse problem than cmd.exe interpreting forward slashes as command options, as it can occur in more situations. I think we should use forward slashes throughout. I expected something like that. OK, the -b version uses forward slashes only. It's stupid and less efficient - not that it matters on novadays machines. The command interpreters seem to tolerate forward slashes for an exact-match path. I don't understand why you made these changes with a separate target for each data subdirectory. This is not clean enough IMO. OK, I just got it: copy src/* dest The syntax of the command is incorrect. I still don't think separate targets is acceptable though. I can at least apply the rest of the patch though. Now applied without the install changes; thanks for the patch. BTW you missed data/templates/files. Wouldn't using a single xcopy be more sensible to reduce bugs and maintenance? I've now committed to master an install target similar to your first patch, but using xcopy for data. I used a DIRSEP variable: $(CP) plugins$(DIRSEP)*.dll $(DESTDIR)/lib I think because 'copy' takes /y switches the first path it sees must not use forward slashes. They are OK in the destination though. Please test. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Remove MSYS dependency of Geany on Win~1
On 25/06/2012 20:33, Dimitar Zhekov wrote: Hi. Here is a small diff (for makefile.win32 and src/makefile.win32 only) that makes Geany 1.22 Win~1 compilation and installation independent of MSYS. Usage: C mingw32-make -f makefile.win32 C mingw32-make -f makefile.win32 install Tested with mingw, cmd/tcc. Should work with MSYS make too... but I had to use backslashes in the install: target, and am not sure if/how MSYS make escapes them. Win~1 fully supports forward slashes internally, but the command interpreters have only partial support. MSYS does treat backslashes as escapes, and I think that is a worse problem than cmd.exe interpreting forward slashes as command options, as it can occur in more situations. I think we should use forward slashes throughout. Unfortunately I can't get your patch to apply against current Git, but I'm not sure why (see attached file for errors). Can you/someone test if it applies (maybe there's something weird happening on my Windows setup)? old~/no-MSYS-1.22.diff:8: trailing whitespace. MD = mkdir old~/no-MSYS-1.22.diff:9: trailing whitespace. CP = copy /Y old~/no-MSYS-1.22.diff:12: trailing whitespace. MAKE = mingw32-make old~/no-MSYS-1.22.diff:23: trailing whitespace. $(MAKE) -C tagmanager/mio -f makefile.win32 old~/no-MSYS-1.22.diff:24: trailing whitespace. $(MAKE) -C tagmanager -f makefile.win32 error: patch failed: makefile.win32:14 error: makefile.win32: patch does not apply error: patch failed: src/makefile.win32:77 error: src/makefile.win32: patch does not apply ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Remove MSYS dependency of Geany on Win~1
On 26/06/2012 20:06, Dimitar Zhekov wrote: MSYS does treat backslashes as escapes, and I think that is a worse problem than cmd.exe interpreting forward slashes as command options, as it can occur in more situations. I think we should use forward slashes throughout. I expected something like that. OK, the -b version uses forward slashes only. It's stupid and less efficient - not that it matters on novadays machines. The command interpreters seem to tolerate forward slashes for an exact-match path. I don't understand why you made these changes with a separate target for each data subdirectory. This is not clean enough IMO. BTW you missed data/templates/files. Wouldn't using a single xcopy be more sensible to reduce bugs and maintenance? Unfortunately I can't get your patch to apply against current Git, but I'm not sure why (see attached file for errors). Can you/someone test if it applies (maybe there's something weird happening on my Windows setup)? The original patch had CR+LF line endings, the attached -a version uses LF. But we should try -b anyway. The problem turns out not to be line endings, that was just git warning noise. The problem was something else, still don't know why. I can now apply the -b version, but I had to add 'src/' to the second file in the diff - are you manually concatenating diffs? Wouldn't it be better to use 'git diff'? Note that using a cmd-style shell with SHELL defined to a *nix style shell fails, no matter if makefile.win32 is patched or not. That's a problem of GNU make under Win~1 trying to be too smart, and ending up with things like CreateProcess (copy, ...) error 2. :) I don't get the last part above, but I think this is by design: The program used as the shell is taken from the variable SHELL. If this variable is not set in your makefile, the program /bin/sh is used as the shell. ... Unlike most variables, the variable SHELL is never set from the environment. This is because the SHELL environment variable is used to specify your personal choice of shell program for interactive use. It would be very bad for personal choices like this to affect the functioning of makefiles. from http://www.gnu.org/software/make/manual/html_node/Choosing-the-Shell.html#Choosing-the-Shell Also, I think you /can/ join command lines with a trailing backslash (but your patch change is still better to have STLIBS anyway): http://www.gnu.org/software/make/manual/html_node/Splitting-Lines.html ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Remove MSYS dependency of Geany on Win~1
On 19/06/2012 22:25, Matthew Brush wrote: On 12-06-19 10:12 AM, Dimitar Zhekov wrote: Hi, Now that 1.22 is out, how about removing the MSYS build dependency under Win~1? I tried to compile Geany with the default MinGW make, without any MSYS, and there were some easily fixable problems: cd foo $(MAKE) -f makefile.win32 cd ..\.. does not work, probably requires some sh. But if we depend on GNU make (which seems to be the case, since we're using ifdef/else/endif), it's easier and shorter to: $(MAKE) -C foo -f makefile.win32 OK. Linking does not work, because the stock make supports \ only for variables, not commands. But that's even easier to fix: OK. There is also some inconsistency: CP = copy, but cp -r and cp at the end of makefile. The install target can be rewritten as several -$(MD) and non-recursive $(CP) commands, and no MSYS will be required. Sigh. Is there no equivalent of 'cp -r' on Windows? RFC. If we consider this worthy, I can make the required changes. Sounds good. I have started working on this before: https://gist.github.com/1494603 It builds but isn't perfect, like it doesn't do the icon/resource stuff, and doesn't use win32-conf.h properly, but it might be useful for a starting point. It's what I use for testing Geany on Windows. I did respond to this before (maybe with a laundry list of suggestions), but off the top of my head, the main ones for me are: * it doesn't show the commands, so devs won't notice e.g if CFLAGS aren't correctly set * no support for building from MSYS I haven't actually tested the makefile, but the idea of a single file is appealing, e.g. we can avoid duplication for GTK_CFLAGS, etc. On 20/06/2012 19:04, Dimitar Zhekov wrote: That looks like an entirely new makefile. I was able to compile Geany with small (msys compatible) changes in the existing makefiles, and setting MAKE to mingw32-make in localwin32.mk... Yes, probably we should set MAKE=mingw32-make instead of 'make'. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] [geany/geany] Question on - 7b3b65: Add workaround for users with an invisible selection style
On 07/06/2012 00:11, Lex Trotman wrote: Those people should now fix their color schemes. I could lower the severity to an info message. Don't think it makes much difference, up to you. But we should add it Info messages are not shown on the console with recent GLib AIUI. to the incompatibilities section of NEWS iaw http://thread.gmane.org/gmane.editors.geany.devel/6590/focus=6594. There isn't an incompatibility here due to the workaround if I use g_info instead of g_warning (which I probably should have done anyway). I'll add with anything else mentioned in the thread. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] [geany/geany] Question on - 7b3b65: Add workaround for users with an invisible selection style
On 05/06/2012 06:10, Lex Trotman wrote: On 5 June 2012 01:30, Nick Treleavennick.trelea...@btinternet.com wrote: On 04/06/2012 07:59, Lex Trotman wrote: Based on your answer on my ML question is this necessary? Now that we always apply the settings, a selection style of false,false will reset to the Scintilla default The default is to show the selection by changing the background to light gray and leaving the foreground the same as when it was not selected. So false, false shouldn't be invisible. Of course it might be invisible on particular backgrounds, but so might c0c0c0. Or have I still misunderstood something? Try it yourself - here I get an invisible selection if neither override is set. Possibly a Scintilla bug - the Scintilla default is not restored on calling SCI_SETSELBACK when the first argument is 0/FALSE. Hmm, the code certainly uses the text fg/bg if the flag isn't true Editor.cxx:2379 so I asked Neil for clarification of the docs which might be read either way, he confirms it is intended this way. So we need to do something like this. Since we are in string freeze I guess all we can do is a fixed value, we can't add any user settings or change the message. Debug messages are not translated. The string freeze only prevents us changing translated strings. I guess we need to advertise the change of colour scheme behavior for the release, otherwise we will get lots of complaints about the messages (like Matthews) when people use customised schemes without one of the flags being true. I doubt we'll get lots of complaints - most people don't even read console messages. Those people should now fix their color schemes. I could lower the severity to an info message. Regards, Nick ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] [geany/geany] Question on - 7b3b65: Add workaround for users with an invisible selection style
On 04/06/2012 07:59, Lex Trotman wrote: Based on your answer on my ML question is this necessary? Now that we always apply the settings, a selection style of false,false will reset to the Scintilla default The default is to show the selection by changing the background to light gray and leaving the foreground the same as when it was not selected. So false, false shouldn't be invisible. Of course it might be invisible on particular backgrounds, but so might c0c0c0. Or have I still misunderstood something? Try it yourself - here I get an invisible selection if neither override is set. Possibly a Scintilla bug - the Scintilla default is not restored on calling SCI_SETSELBACK when the first argument is 0/FALSE. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Selection and colour schemes
On 02/06/2012 07:27, Lex Trotman wrote: Hi All, I just pushed a fix for switching colour schemes from say dark to default leaving selection unchanged. But in reality the code at highlighting.c doesn't actually do what it advertises, the comment says to override default which matches the documentation, but in fact what the code does is to change from what is currently set. Of course if you are changing from default to another colour scheme thats ok, but if you are changing from a colour scheme that has changed this setting then what is set won't be default. I think the code should be: if (common_style_set.styling[GCS_SELECTION].bold) SSM(sci, SCI_SETSELFORE, etc... else SSM(sci, SCI_SETSELFORE, whatever the default is) but I can't find where to get the real default value and have run out of time. I've committed a fix, and also restored using the default foreground color for selections, so the style information can still be seen. You're right to enable the default background color, as otherwise with no overrides users can't see any selection. The default is explained here: http://www.scintilla.org/ScintillaDoc.html#SCI_SETSELFORE SCI_SETSELFORE(bool useSelectionForeColour, int colour) SCI_SETSELBACK(bool useSelectionBackColour, int colour) You can choose to override the default selection colouring with these two messages. The colour you provide is used if you set useSelection*Colour to true. If it is set to false, the default styled colouring is used and the colour argument has no effect. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] tagmanager changes
On 02/05/2012 05:46, Lex Trotman wrote:
Hi All,
To summarise since the thread has several subthreads.
1. Tagmanager Understandability
a. I generated the doxygen documentation for tagmanager, it works if
you set recursive, but didn't help much:
- if its not OOP why does it say things like TMWorkspace is derived
from TMWorkObject and similar?
documentation bug IMO
- its not clear how it all goes together, the workspace contains
global tags and work_objects, or is that files and whats the
workspace work objects are document tags. global tags explained in
geany's manual.
difference between source_file and file_entry?
It doesn't look like tm_file_entry_ is really used.
- similarly whats the difference between symbol and tag?
tm_symbol_ doesn't seem to be used.
2. Ability to expand tagmanager to handle names declared in lexical
scopes (not to be confused with struct/class scopes). Here is the
example again with some numbers so I can refer to them
{ struct a o; struct a p;
o./* struct a members */
{ struct b o;
o./* struct b members */
p./* struct a members */
}
o./* struct a members */
p./* struct a members1 */
{ struct c o;
o./* struct c members */
p./* struct a members2 */
}
o./* struct a members */
p./* struct a members */
}
a. yep, tries use more memory than an array, the usual speed/space,
pick one, tradeoff :)
b. @Nick, when you say sort by scope then name, are you wanting to
have an entry in the table for each declaration of the name?
no
- If so this makes the array much bigger to search and your search
speed depends on size, and it doesn't get you anything, you can't
search by scope since you don't know if the name is declared in the
scope you are in or an outer scope compare p at1 and2
- having a single name array which then points to scope info for the
name is a viable approach (disclosure, thats how I'm doing the symbol
table for a language I'm developing) but the table being searched is
usually larger than if you have nested arrays. Being smaller these
are faster to search if the search isn't O(1), hence the suggestion of
trie instead of bsearch.
the gain in simplicity makes a bigger array to search worth it.
Remember, global tags aren't included in the workspace array of
tagmanager, so we're not talking a big number of tags, and we have o(log
n) searching.
4. Ctags parsers
Agree with Nick that the parsers are usable, but if we start modifying
them to handle local declarations then they will be totally
incompatible with the Ctags project so I guess it doesn't matter other
than for getting languages we don't currently parse.
ctags c.c already parses local tags
5. Overloaded symbols
Since Colombans patch, overloaded symbols are now stable for all
practical code (I think theoretically it could get confused if the
overloads are on the same line but thats unlikely enough to ignore for
human generated code)
If you're talking about master, I think I still experienced wrong
parenting on reparsing when removing lines.
6. Moving functionality from symbols.c to tagmanager
a. Since its the 100th anniversary of the Titanic sinking, I think
that shuffling the deckchairs is an apt analogy, the functionality
has to be somewhere, its only useful to move it if the destination
significantly reduces the effort required.
I don't think I suggested moving functionality. I wondered whether TM
could help make symbols.c less complex. I would need to understand the
complexity to know whether this is appropriate or not.
Titanic comment is bizarre. My suggestion could equally apply to
ctagsmanager.
___
Geany-devel mailing list
Geany-devel@uvena.de
https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] tagmanager changes
On 07/05/2012 17:04, Nick Treleaven wrote: On 02/05/2012 05:46, Lex Trotman wrote: - its not clear how it all goes together, the workspace contains global tags and work_objects, or is that files and whats the workspace work objects are document tags. global tags explained in geany's manual. difference between source_file and file_entry? It doesn't look like tm_file_entry_ is really used. - similarly whats the difference between symbol and tag? tm_symbol_ doesn't seem to be used. Also, tm_project_ is not used either. 2. Ability to expand tagmanager to handle names declared in lexical scopes (not to be confused with struct/class scopes). Here is the example again with some numbers so I can refer to them b. @Nick, when you say sort by scope then name, are you wanting to have an entry in the table for each declaration of the name? no - If so this makes the array much bigger to search and your search speed depends on size, and it doesn't get you anything, you can't search by scope since you don't know if the name is declared in the scope you are in or an outer scope compare p at1 and2 - having a single name array which then points to scope info for the name is a viable approach (disclosure, thats how I'm doing the symbol table for a language I'm developing) but the table being searched is usually larger than if you have nested arrays. Being smaller these are faster to search if the search isn't O(1), hence the suggestion of trie instead of bsearch. the gain in simplicity makes a bigger array to search worth it. Remember, global tags aren't included in the workspace array of tagmanager, so we're not talking a big number of tags, and we have o(log n) searching. Oops, forget the global/workspace division, we still need to search global scopes. But I don't know why you think this is too slow. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Request: multithreaded tag generation?
On 07/11/2011 15:35, Harold Aling wrote: I recently switched from GeanyPRJ to Gproject. Since Gproject doesn't support multiple open projects I have to switch between projects, but it takes up to 4 minutes to close one project and open another. A project consists of roughly 1000-2000 php-related files. The Generate tags for all project files causes this massive delay, but I really need that feature. At work I have a 2-core CPU, where 1 is completely idle and on my desktop at home there are 5 cores are doing nothing while generating tags. Can't they be utilized to speed up the tag generation? As a workaround I wonder if the plugin could generate a temporary global tags file (with geany -g). This would be a separate thread. Then tell Geany to load the tags file - this is faster in Geany Git as it avoids resorting all global tags. We would need to add a way to remove global tags though, but this is probably straightforward. Nick ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] tagmanager changes
On 29/04/2012 14:07, Lex Trotman wrote: On 29 April 2012 22:07, Nick Treleavennick.trelea...@btinternet.com wrote: On 27/04/2012 06:30, Lex Trotman wrote: I don't see this myself, I see some complicating issues which could be resolved (and I'm willing to work on them), but generally a sound design for what it provides and for extra things we may want to add. Maybe my problem is simply the attempt to simulate object orientation and inheritance in C makes it look complicated for a spoilt C++ programmer :) Eh? Tagmanager isn't OOP. I expect the design is better in some respects (and to be fair I didn't look for better things), but finding a tag based on its name is something we are always going to want to be fast. Even for scope completion, you still need to lookup a tag structure from a name string. So I think we will always want a sorted array of tags per document that we can bsearch (or something equally fast). Yes, we could have one name table and then prune the results to the scope required, or a name table per scope (which makes the tables smaller and simple searches faster). It seems to me that if we supported proper scope completion then we could still have one array per document, sorted first by scope, then by name. I don't know, but we still need fast tag lookup based on name. If O(n) scope lookup is too slow, we will need additional data structures arranged differently, but whatever we have should have something like O(log n) lookup for names as this is by far the most common operation. Agree, name lookup should be as fast as possible, O(log n) what about O(1) like compilers :) Well since we can't use hashes due to the need to identify prefixes, a radix trie seems the best since it can be as fast as a hash ie two passes through the key, and that gives a sub-trie that begins with the prefix. But as you say thats orthogonal to the question of scope layout. I only know about a simple trie, but wouldn't that use a lot more memory? I don't think we want name lookup 'as fast as possible', just no slower than we have already. * tags (and most types) are reference-counted (so they aren't necessarily duplicated, e.g. in the multicache backend); I don't really understand src/symbols.c since the real-time parsing change, so don't really understand why this is needed. Blame C++ and overloaded names I think. I looked at the thread about that, and from what I could tell, the problem was for reparsing unsaved files. Wasn't the order OK for files that have just been saved? (Also I don't follow what that has to do with reference counting). Hmm, you're right, looks like Colomban is duplicating tags in nested caches, so all searches are just one cache, but in fact they are not duplicated, but reference the same tag using a reference count to tell when to destroy it. I am not sure where the reference to real time parsing comes from? Sorry, I meant reparsing not real time updates. But is your problem with overloaded symbols OK when the file is parsed after saving, i.e. the order is only wrong on reparsing? ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] tagmanager changes
On 29/04/2012 15:42, Colomban Wendling wrote: * it support asynchronous parsing (though not concurrent parsing); What's the difference? Also, what does it buy us? What I meant when saying it's asynchronous but not concurrent is that it supports launching the parsers in a separate thread, but it cannot launch several parsers at once. This is because CTags parsers aren't thread-safe (have a lot of global states and no locks). What asynchronous parsing gives us is quite simple: no blocking. This means that a slow parser (like e.g. the HTML one on Windows before you changed the regex library) wouldn't lock Geany. Same for project plugins that want to parse thousands of files: this would still take a long time, but Geany would be still usable in the meantime. Ok. It seems a good idea to support this, but for parsing tags in open documents it doesn't seem particularly useful, as this ought to be fast. The regex problem was unusual IMO and due to an old version of GNU regex. I don't understand why tagmanager has to be replaced, why not just replace the parts you want to improve? Rewriting it is likely to lead to a new set of bugs and be hard to review and merge changes from master. I think Lex and Matthew probably summarize it quite correctly: it's not that TM is bad or has to be replaced; it is that (I'm) unable to understand it enough to fix anything in it, like scope completion. Maybe it's only me, but I tried hard :) And the only reason why maybe TagManager would need replacement/large changes is asynchronous parsing. I'm not sure how hard it would be to get it with TM -- but again, maybe it's only that I don't understand it enough. Not sure about this, but I think the only messy part of TM is when it updates the workspace array, which I think could be removed (I'll explain this in another reply). * there is 2 backends as of today: - a simple one that is simple and that doesn't waste (too much) memory, but that searches in O(n); Linear searching seems like a bad idea when we currently have O(log n). Removing x tags is O(n * x) by the look of it. I agree it's not interesting regarding performances, and this backend isn't meant for it. I needed an implementation for early testing, so I wrote it simple. And it showed useful for testing later on too, since because of it's simplicity it isn't bugged -- just slow :) Ok. BTW, how does TagManager do fast searches? I always though it did a sorting with new attributes each time they changed, so in such situations it's even worse than O(n), isn't it? For searching, it doesn't do any sorting ever. For adding tags the work object (i.e. document tags) have to be sorted, which I think is good, but also the workspace tags are currently resorted, which I think may be a bad design. - a multi-cache one that, as its name suggests, maintains multiple caches (sorted tags arrays). it uses a little more memory and is slower on insertion since it maintains several sorted lists, but a search on an already existing cache is very fast. Won't this be slow for adding many tags at once? How is this design better than tagmanager? Well, adding a tag would require a bsearch() on each cache, yes. However, adding tags is mostly done in a separate thread (if async parsing it used), so it can be less of a problem. I haven't studied your design, but I would prefer that any design is efficient on all threads, so the user's computer can use remaining performance for compiling whatever else they want. Also, what about global tags? Those can add a lot of tags all at once. And a search is simply a bsearch() (O(n log n), right?) given the cache already exists. If the cache doesn't exist, it has to be created so yeah on the first search it's slow. If it can be slow enough for the user to notice this is probably bad. I don't see why having two is better. The memory overhead for a pointer array is not much vs. the size of the tag structures. Fast searching is important. Multiple backends isn't really useful probably. As said above, the first was mostly a testing one, and I wanted to be able to keep both during development for better testing. At one point I also though maybe there could be some backends optimized for particular situations, like fast search, fast insertion, low memory consumption, etc., but I don't see much use for this anymore. Ok. * tags (and most types) are reference-counted (so they aren't necessarily duplicated, e.g. in the multicache backend); I don't really understand src/symbols.c since the real-time parsing change, so don't really understand why this is needed. BTW here I meant reparsing document tags without saving, sorry for any confusion. It's not strictly needed, but it makes some memory management easier, and fits well with GTK memory management. And this last point helps a lot to maintain the GtkTreeStore on src/symbols.c, now tags are updated and not
Re: [Geany-devel] tagmanager changes
On 29/04/2012 15:47, Colomban Wendling wrote: Le 26/04/2012 18:53, Nick Treleaven a écrit : On 26/04/2012 16:02, Nick Treleaven wrote: On 24/04/2012 22:31, Colomban Wendling wrote: * it uses the same tag parsers tagmanager used, in ctagsmanager/ctags; BTW this is a good idea to clearly separate CTags from tagmanager. If this change can be applied separately, perhaps it could be merged into master. It should be quite easy -- though it won't still be a vanilla CTags of course, my own isn't either (yet?). I just thought it was a separate change from the TM rewrite. We have a lot of changes from CTags e.g. new languages and improvements to existing parsers (and even CTags itself) which IMO we can't throw away. It is possible to merge some changes from CTags but for e.g. c.c this has become pretty difficult. I tried this once but it ended up breaking the parser and I didn't work out why. IMO the biggest issue with CTags is that it isn't really maintained and they rarely accept patches even when it fixes an important bug (e.g. a fix to regex callback parsing with ignored matches). For avoiding resorting of workspace tags when only reparsing one source object, we can remove the source object's old tags merge the new tags. This is all O(n) for the workspace array. I haven't looked into implementing this yet because now it's clear you're working on a competing change. Another option is to remove the workspace array altogether and just have Geany collate tags from each (sorted) source object when needed. Not sure the implications of this yet but it would simplify tagmanager. Well, tagmanager needs to know all tags to perform e.g. scope completion beyond file's boundaries. And for search too, it would need us to pass it everything, or to perform a search on each document's tags and then merge the result. Doesn't sound sensible at a first glance, but maybe I'm missing something. It comes to a trade off between merging/sorting results from multiple tag arrays and merging/sorting the whole workspace array even when only one document is reparsed. Actually I suppose the first one can cause lags which the user could notice if there are a lot of results, whereas the second one only causes problems on reparsing, which is less often than searching. I'm undecided. The second one can be more serious if the calling code doesn't handle e.g. save all specially and the workspace array size is quite big. The special handling is to not update the workspace array until all documents have been parsed, then to resort it. If it's only save all, we can cope, but there may be other ways this can happen. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] tagmanager changes
On 27/04/2012 06:30, Lex Trotman wrote:
[...]
I don't understand why tagmanager has to be replaced, why not just replace
the parts you want to improve? Rewriting it is likely to lead to a new set
of bugs and be hard to review and merge changes from master.
One of the problems with tagmanager is its complexity, leading to
considerable wariness on the part of many of us about changing it
since we don't understand what we might break.
I don't see this myself, I see some complicating issues which could be
resolved (and I'm willing to work on them), but generally a sound design
for what it provides and for extra things we may want to add.
Actually documenting the overall structure of tagmanager and how it is
supposed to work would be a good thing, whats a workspace? what is it
meant to represent, how are scopes represented? etc.
Isn't it clear from the data structures? Look at TMWorkspace. Scopes and
other tag metadata is the same as CTags. Obviously if we had at-a-glance
overall documentation that would be good.
One confusing thing is that a TMTag can be used for an actual tag or for
a file. Probably that could be cleaned up.
- a multi-cache one that, as its name suggests, maintains multiple
caches (sorted tags arrays). it uses a little more memory and is
slower on insertion since it maintains several sorted lists, but a
search on an already existing cache is very fast.
Won't this be slow for adding many tags at once? How is this design better
than tagmanager?
Perhaps Colomban could confirm, but my first thought is that this is
for nested scopes.
I expect the design is better in some respects (and to be fair I didn't
look for better things), but finding a tag based on its name is
something we are always going to want to be fast. Even for scope
completion, you still need to lookup a tag structure from a name string.
So I think we will always want a sorted array of tags per document that
we can bsearch (or something equally fast).
Also, I've probably sounded quite harsh on Colomban's design, but I'm
commenting on what I think is important. I am genuinely interested in
why his design decisions are better. It's a lot to take in all at once,
so probably needs some explanation. Sorry if I didn't make that clear.
How does tagmanager handle nested scopes, or how would it need to be
modified to do so, considering the example (in C)
{ struct a o; struct a p;
o./* struct a members */
{ struct b o;
o./* struct b members */
p./* struct a members */
}
o./* struct a members */
p./* struct a members */
{ struct c o;
o./* struct c members */
p./* struct a members */
}
o./* struct a members */
p./* struct a members */
}
How much needs to be changed in tagmanager so that the right
autocompletes can be provided at each comment? (assuming c.c is
taught to parse local variables of course)
I don't know, but we still need fast tag lookup based on name. If O(n)
scope lookup is too slow, we will need additional data structures
arranged differently, but whatever we have should have something like
O(log n) lookup for names as this is by far the most common operation.
* this backend abstraction might be really overkill, and maybe we
could do better without it?;
I don't see why having two is better. The memory overhead for a pointer
array is not much vs. the size of the tag structures. Fast searching is
important.
It is but is it flexible enough to be expanded to nested scopes.
see above.
* tags (and most types) are reference-counted (so they aren't
necessarily duplicated, e.g. in the multicache backend);
I don't really understand src/symbols.c since the real-time parsing change,
so don't really understand why this is needed.
Blame C++ and overloaded names I think.
I looked at the thread about that, and from what I could tell, the
problem was for reparsing unsaved files. Wasn't the order OK for files
that have just been saved? (Also I don't follow what that has to do with
reference counting).
I don't really see what the problem understanding it is. I thought scope
completion was just tm_workspace_find_scoped and related functions, not some
tagmanager-wide problem.
I think the fact that this isn't clear is the problem :)
If I'm following you correctly, I think you're saying the design needs
to change, which I accept may be true. What I was saying was that
understanding the existing code for scope completion is not really that
complex.
Regards,
Nick
___
Geany-devel mailing list
Geany-devel@uvena.de
https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] tagmanager changes
On 24/04/2012 22:31, Colomban Wendling wrote: Le 17/04/2012 18:20, Nick Treleaven a écrit : Sorry for the long delays -- and also small activity -- recently. I have/had a lot of non-Geany stuff to do and stuff, the whole story, you know. No problem. I finally committed it and pushed it so you can see it, comment, blame, flame more: see https://github.com/b4n/geany/tree/wip%2Fctagsmanager A few points, as they comes to my mind: * it support asynchronous parsing (though not concurrent parsing); What's the difference? Also, what does it buy us? * it is under the new ctagsmanager/ directory; * it uses the same tag parsers tagmanager used, in ctagsmanager/ctags; * all types have different names than the tagmanager ones, though currently the API is almost an 1:1 mapping -- and that's maybe a huge mistake?; I don't understand why tagmanager has to be replaced, why not just replace the parts you want to improve? Rewriting it is likely to lead to a new set of bugs and be hard to review and merge changes from master. * there is 2 backends as of today: - a simple one that is simple and that doesn't waste (too much) memory, but that searches in O(n); Linear searching seems like a bad idea when we currently have O(log n). Removing x tags is O(n * x) by the look of it. - a multi-cache one that, as its name suggests, maintains multiple caches (sorted tags arrays). it uses a little more memory and is slower on insertion since it maintains several sorted lists, but a search on an already existing cache is very fast. Won't this be slow for adding many tags at once? How is this design better than tagmanager? Given my global tags merge change (see below), I think tagmanager needs some changes to avoid sorting the workspace tags array each time tags are updated, but this is certainly doable. In practice I haven't yet tested anything big enough to see any difference in performances between these two backends, and that's something that probably isn't worth bothering about for now. * this backend abstraction might be really overkill, and maybe we could do better without it?; I don't see why having two is better. The memory overhead for a pointer array is not much vs. the size of the tag structures. Fast searching is important. * tags (and most types) are reference-counted (so they aren't necessarily duplicated, e.g. in the multicache backend); I don't really understand src/symbols.c since the real-time parsing change, so don't really understand why this is needed. * tag matching/finding is done using ctm_data_backend_find() (or a convenience wrapper), which takes 2 functions for performing the search: - a sort function, used to, heh, sort the tags to search and/or the resulting list (the simple backend should use it to sort the result; and the multi-cache backend uses it to sort the caches) It doesn't sound a good idea to have to sort the haystack on each lookup. Lookup should be very fast, like tagmanager. Adding/removing tags is always less important because real-time updates can be disabled anyway. - a match function, use to check whether a tag should be included in the results. Like the sort function it returns a strcmp()-like value, with the only difference that it probably returns 0 for more tags. It is somewhat similar to what tagmanager did, but it's more flexible -- and maybe complex, though many sort/match functions would already be provided. * no pruning is done yet, so there is duplicate in the results; * there is an (almost) working scope completeion implementation; * ... I don't see anything to add, so I'll stop here :) All this isn't of course written in stone: if we already redo a lot of things, let's get something nice -- though IMO we'll always be better than with tagmanager, since each time I wanted to touch it it took me hours, and sometimes I even haven't been able to fix the problem (thinking of e.g. scope completion...). I don't really see what the problem understanding it is. I thought scope completion was just tm_workspace_find_scoped and related functions, not some tagmanager-wide problem. Also, later in the thread he says that performance problems with resorting global and workspace tags cannot be fixed with the design of tagmanager. I've been working yesterday on improving this significantly by merging the new tags each time instead of resorting *all* the tags. I hope to commit this in the next few days. Cool! I haven't done any profiling on either tagmanager or may new attempt, so I can't tell what's actually slow, but any improvement is good to have :) I finished the global tags merging change: https://github.com/geany/geany/commit/712cdd6aa0005f65bd77b15c34cd29067864628d IMO this should make loading global tags fast enough, even for heavy use, as the whole array is now only traversed once. For avoiding resorting of workspace
Re: [Geany-devel] tagmanager changes
On 26/04/2012 16:02, Nick Treleaven wrote: On 24/04/2012 22:31, Colomban Wendling wrote: * it uses the same tag parsers tagmanager used, in ctagsmanager/ctags; BTW this is a good idea to clearly separate CTags from tagmanager. If this change can be applied separately, perhaps it could be merged into master. For avoiding resorting of workspace tags when only reparsing one source object, we can remove the source object's old tags merge the new tags. This is all O(n) for the workspace array. I haven't looked into implementing this yet because now it's clear you're working on a competing change. Another option is to remove the workspace array altogether and just have Geany collate tags from each (sorted) source object when needed. Not sure the implications of this yet but it would simplify tagmanager. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Standard Tags files
On 25/04/2012 03:35, Lex Trotman wrote: Hi All, Nick has removed the instructions on re-building tags.c99 which is ok. But it made me think, should we provide a tags.c11 as well and how would it be best to choose between them? IIUC C11 adds a significant number of libraries, so extra tags could be good. But C11 is fairly new and I don't know how compliant the various libraries are yet, any comments from C11 users? I don't think we should include both with Geany, but maybe we can just distribute a c11.tags file. In fact it should probably just be called c.tags without the version, like pascal.tags. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Small additions to stash
On 05/04/2012 19:02, Dimitar Zhekov wrote: New stash_group_free_strings(), which frees any strings and string arrays in a group. Much easier than to track them individually, as in Geany (though if someone is willing to track the keyfile_groups prefs free-s and remove them, a call may be included in configuration_finalize). Probably it should be named more generally, in case we support e.g. null-terminated integer lists. Maybe stash_group_free_settings(). Renamed and changed the description. Updated patch attached. Thanks, applied. I added a note to the docs that it's not called by stash_group_free(). Ideally we would be using the function in Geany somewhere as an example. Regards, Nick ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Small additions to stash
On 04/04/2012 19:58, Dimitar Zhekov wrote: Hi, How about some $subject? Namely: Include stash_group_set_use_defaults() in plugin StashFuncs. I'm not using it any more, but it's a nice thing to have. I'm not convinced we should support that publicly. It puts a burden on the implementation for an unusual use case. New stash_group_free_strings(), which frees any strings and string arrays in a group. Much easier than to track them individually, as in Geany (though if someone is willing to track the keyfile_groups prefs free-s and remove them, a call may be included in configuration_finalize). Probably it should be named more generally, in case we support e.g. null-terminated integer lists. Maybe stash_group_free_settings(). Regards, Nick ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Gathering Release 1.22 incompatibilities
On 20/03/2012 04:46, Lex Trotman wrote: +Filetypes +* Move all named styles into color schemes and keep only mappings in the + filetypes files. Filetypes should no longer contain styling information, + except `filetypes.common` which contains the default color scheme. Breaks + compatibility with old filetypes files. I think the filetypes.* files compatibility was not broken. Rather that overriding filetypes style defaults will break color schemes. +Plugin API +* Rename signal `project_dialog_create` to `project_dialog_open` and + add new signal `project_dialog_close`. Increments plugin ABI so all + plugins need re-compilation even if they don't use the signal. If compiling doesn't catch any new errors, I don't think the ABI should have been broken. Maybe it's worth reverting the ABI break? +User Interface +* Keybinding Ctrl-t has been re-purposed from transpose line to + go to tag. Transpose line should be less used now the move line + commands are available. I've now committed a workaround - see the latest 2 commits. Existing users should see no change on upgrading Geany, even if they never edited any keybindings. New users get the new keybinding defaults. Also, shouldn't the important/breaking things be in the same group at the top? Otherwise when we add the other changes they won't stand out. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Gathering Release 1.22 incompatibilities
On 19/03/2012 01:41, Lex Trotman wrote: On 19 March 2012 12:12, Matthew Brushmbr...@codebrainz.ca wrote: On 12-03-18 05:20 PM, Lex Trotman wrote: Hi All, I recently ran into Nicks change of the default keybindingctrl-t from transpose to gototag. This made me realise we need to keep a list of incompatibilities to add to the release notes when 1.22 is released. I don't know if I'd consider changing a default keybinding an incompatibility as such. IMO, unless something breaks as a result of an upgrade, it's not really incompatible. Its incompatible with users fingers, and thats a *major* incompatibility. :) Agree. I might be able to improve this a bit with a workaround, I'll investigate. Basically anything that changes the UI operation (eg moved menu items, keybindings, the new colourschemes dialog etc). should be mentioned so users are not surprised. You can get some *quite* nasty bug/ML reports if such changes are not brought to the users attention. It doesn't have to be in incompatibilities, there can be a changed UI section as well, but I thought just one section would be better. Agree, anything confusing/annoying should be clearly mentioned at the top of the release notes IMO. I would have thought that anything incompatible would have been forgotten by then unless we keep a running list, at the moment all I can think of is the ctrl-t and themes, but I am sure there are others. The only real incompatibilities I can think of are the filedefs/color schemes, changes to the plugin API, and the GTK+ version bump. Yeah, plugins is an important one. I may have missed some things but offhand is this just the project properties signal names that got broken API-wise? The list also saves Git blaming to try to see what made the change and if it is deliberate or not. So any suggestions on how we should gather these? and of any more of them. We could, at release-time, just manually scan the ChangeLog (and/or Release Notes) and add an asterisk to each item that changes defaults or breaks compatibility, If its obvious, but for example Nicks commit comment actually said it didn't affect users (not picking on Nick, that just happened to be the one I just fell over), Yeah, unfortunately I forgot keybindings.conf is only saved after the first edit. (IIUC, commit messages shouldn't be edited after they've been pushed). so it wouldn't be obvious. Commit comments often don't mention it. So I think gathering these things as we go is also important. ... with a note at the bottom of the list to explain what the asterisk means. Thats one way of presenting it yes, but I think it is best to make a separate list in a separate section of the release notes so it has a *chance* of being read by at least *some* users. Little *s are too easily overlooked. Agree, I think Colomban's idea of adding incompatible/important changes to the NEWS file as we go, and at the top, would work well. Regards, Nick ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] [geany/geany] 7cc443: Don't append file truncation warning if file doesn't exist - incorrect for remote files
On 01/02/2012 23:05, Lex Trotman wrote: On Thu, Feb 2, 2012 at 3:51 AM, Nick Treleaven nick.trelea...@btinternet.com wrote: On 31/01/2012 23:09, Lex Trotman wrote: Confusing the user is also harm. A (paranoid) user may worry some other file got truncated. Ok, better put the filename in the message then, but preventing the message in the case where it is probable that actual harm has been done is really bad. It would still confuse the user. Why tell the user something worrying that may well not be the case? So why did you add a may message in the first place :) I still contend that it is more bad to hide a possibly valid damage warning than to cause consternation by a possibly invalid message. BTW do you agree/disagree with: I'd agree that I (like you) would *expect* it, but sadly I don't think thats the case in the real world. I'm not sure we can assume all users are fully cogniscent of the fact that things are remote, NFS and samba mounts do a good job of hiding remoteness, but they still fail more often (in my experience) than out of disk happens. And to a user plugging in a NAS device is just the same as plugging in a USB disk isn't it? After all it is attached to the machine? Look its my g: drive! Come to think of it, USB disks can have the plug bumped etc. There are lots of ways for temporary failures to happen long before we get to things as exotic as ssh or ftp connections beneath GVFS mounts OK, you persuaded me. Reverted. Nick ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] [geany/geany] 7cc443: Don't append file truncation warning if file doesn't exist - incorrect for remote files
On 27/01/2012 21:36, Lex Trotman wrote: Anyway, I don't know how to improve the situation. My commit is necessary to avoid confusing users about truncation on a local file that doesn't even No, neither do I, but we do get complaints about truncated files on network shares/gvfs every so often, thats why we added the message. IIRC I added the message because of unexpected truncation of files on any FS that has run out of space. So this change actually prevents the message in its main use-case, where harm may have been done, in favour of a use-case where no harm has been done, and that is bad. Confusing the user is also harm. A (paranoid) user may worry some other file got truncated. When writing to a networked file I would expect users to take more notice of error messages, i.e. they should expect a save error might cause truncation. Nick ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] [geany/geany] 7cc443: Don't append file truncation warning if file doesn't exist - incorrect for remote files
On 26/01/2012 22:32, Lex Trotman wrote: If writing a remote file failed half way because of a network failure, then if the network is still faulty when we g_file_test for a regular file, it will fail because the file doesn't exist, but the partly written file may (is likely to) be truncated. Well, they'll still get a save failed warning with the specific OS error message. Just to check, GFile writes directly to a network file? Anyway, I don't know how to improve the situation. My commit is necessary to avoid confusing users about truncation on a local file that doesn't even exist. Nick ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] encoding combo boxes bug - utility functions
On 26/01/2012 05:21, Lex Trotman wrote: On Thu, Jan 26, 2012 at 3:41 PM, Matthew Brushmbr...@codebrainz.ca wrote: On 01/25/2012 06:45 PM, Lex Trotman wrote: [...] Hi Matthew, [...] utils_is_uri() - good utility function, well named Indeed well named and probably a little clearer that `strstr(uri, ://) != NULL` but that's probably what I'd write if I didn't know Geany had it's own function for this, or I'd use g_uri_parse_scheme() or something. You raise a good point, documentation and awareness of utils I think reading utils.h (and utils.c for specific details) is a reasonable expectation. Often using autocompletion helps if you know the start of the function name. resources. ATM the only documentation produced is for functions in the API, needs a version for the utils and any other generally useful bits. Can doxygen distinguish two sets of doc comments in the one file so we can make a utils documentation set as well as the plugin API? I would guess doxygen can't do that. Also it might be less clear as to what's in the plugin API and what's not. [...] utils_spawn_async() - I think was used more than one place in the past, also hides the messy #ifdef windoze which is good If the GLIB functions don't work (ie. on win32), we should send a bug report/patch upstream, just as we'd do with Scintilla if we found an obvious bug. We shouldn't have our own fixed implementation, IMO (unless it does something else I'm not aware of, or upstream refused the fix). You'll have to ask Enrico (I think) why the win API is used and why builds on windows are synchronous, thankfully most of this was done before I arrived and I only have a vague awareness of the reasons. OTOH I don't know that I like your chances of fixing windows Glib and then it will be in a version we get to a ways in the future so the win interface will have to stay for some time. I'm pretty sure the glib spawn functions do not work, other projects had this problem too. I think it is a design flaw rather than implementation detail, but feel free to research this. FWIW I think utils_spawn_async() is a disaster - it tries to combine async and sync parameters in one function, they are fundamentally different. If we implement async spawn on Windows (like SciTEWin::ExecuteOne()) then the utils_spawn_async() parameters would no longer make sense. Maybe we should deprecate it now because of these issues (it's in the plugin API). [...] utils_make_filename() - reasonable utility function, probably should be used in more places where filename.ext concat is done explicitly I never would've thought to use this function over g_strjoin() and g_build_filename() or something. Actually utils_make_filename() is really just g_strconcat without one separator argument. I don't mind if we remove it. To reply to remaining points from earlier message: On 26/01/2012 02:45, Lex Trotman wrote: utils_slist_remove_next() - local static used one place, agree no reason to exist I decided to remove it. I thought it might get used elsewhere, but it didn't. utils_string_replace() - probably should be static, only used several times in utils itself This was used in editor.c but the code got rewritten differently. I'm not sure that it's worth making it static as this will trigger a rebuild of src/*.o and we might use it sometime. utils_build_path() - g_build_filename() has better portability semantics, should replace utils_build_path() Good point. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] encoding combo boxes bug - utility functions
On 24/01/2012 03:30, Matthew Brush wrote:
I probably don't know 40%+ of Geany's code after casually hacking on it
for well over a year. When reading the code, I have to refer to the
source file for each function called to see what it does, with GTK+ I've
already done this for many cases, and know what it does. When writing
the code, I have to first write it in normal GTK+ and then go through
and make sure I haven't used any functions that are wrapped in the Geany
API/headers and even other static functions in the same file. It sounds
trivial if you are intimate with the source code, but if you aren't it
can make understanding the code you need to understand in order to fix a
bug or add a feature that much harder to follow.
If the function and its parameters are well named this isn't a big problem.
then that's a significant benefit in avoiding temporary variables or
nested expressions, which are harder to read. As I said, if the function
is obvious, there's no harm.
You don't really avoid temp vars, you just put them in another file. And
Eh? You have *one* copy of the temp var, not e.g. 10.
if an expression is only nested one or two levels deep, it's easier (at
least for me) in many cases to read if the code is inline.
For a (fictional) example:
void some_func(void)
{
GError *err=NULL;
if (!g_some_func(..., err))
{
printf (error: %s\n, err-message);
g_error_free (err-message);
}
...
}
is easier for me to read than:
/* misc.h */
#define EZG(...) { ... actual code from above ... }
separate files
/* some.c */
void some_func(void)
{
EZG(g_some_func, ...);
...
}
Even if it saves you repeating that same 5-6 line idiom a thousand times
throughout the source, unless you wrote both pieces of code, or unless
EZG() is in a well know API like GTK+, then it makes the code harder to
read, IMO, which many more people do many more times than writing it.
When have I ever suggested doing that?
___
Geany-devel mailing list
Geany-devel@uvena.de
https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] glade diffs - Re: encoding combo boxes bug
On 22/01/2012 13:01, Nick Treleaven wrote: On 20/01/2012 18:39, Colomban Wendling wrote: @Nick: how did you generate the Glade file for 21cd7bb2139fd67644e5777bb8e6387d34473d09 Add Project overrides for 'Saving files' checkbox options? When I do use Glade 3.8.1 do modify the file it keeps reordering prefs/project dialogs... Just wondering, actually committing the move isn't really harmful -- apart that it renders the diff unreadable, stupid Glade. You're right, sadly my idea of using the same version of Glade doesn't prevent huge unreadable diffs. I committed 'regenerate with Glade 3.8.1' as an experiment, which failed. BTW unnecessarily large diffs are a real problem for another reason - they make a merge conflict much more likely. Ideally Glade would fix this reordering behaviour as it's bound to cause us trouble sooner or later. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] encoding combo boxes bug
On 20/01/2012 21:29, Matthew Brush wrote: @All: I added ui_builder_get_object() to be able to fetch a non-widget (list store here) from prefs.c, but I'm not completely sure it's the prefect fix. If you have any idea on how to improve this, spread them! :) IMO, it'd be better to just move the builder object to the header file (maybe in a suitable struct), so that all files can access it. Then there's no need to add 1 line wrapper functions for every function we use from GtkBuilder's API. This isn't unprecedented, I think there's at least a handful of globals like this in Geany already (even in ui_utils.h). Alternatively, we could add a function called `ui_get_builder()` to get access to the builder to use with GtkBuilder API. Otherwise, it's not too big of a deal, maybe we don't need much more from the GtkBuilder API. I think Colomban's function is fine. I don't understand avoiding adding functions that are obviously useful and cleaner: obj = ui_builder_get_object(name); vs. obj = gtk_builder_get_object(ui_get_builder(), name); The former is easier to read and it's obvious what it does. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
[Geany-devel] glade diffs - Re: encoding combo boxes bug
On 20/01/2012 18:39, Colomban Wendling wrote: @Nick: how did you generate the Glade file for 21cd7bb2139fd67644e5777bb8e6387d34473d09 Add Project overrides for 'Saving files' checkbox options? When I do use Glade 3.8.1 do modify the file it keeps reordering prefs/project dialogs... Just wondering, actually committing the move isn't really harmful -- apart that it renders the diff unreadable, stupid Glade. You're right, sadly my idea of using the same version of Glade doesn't prevent huge unreadable diffs. I committed 'regenerate with Glade 3.8.1' as an experiment, which failed. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] SCSS (Sassy CSS) lexer support
On 17/01/2012 20:36, Lex Trotman wrote: I think if all is needed is to copy SciLexer.h and the new lexer file, plus minor makefile changes, then a patch should be acceptable without upgrading Scintilla. We've done this in the past. Hi Nick, How does this work without a filetype to support the new lexer? And don't we need additions to sciwrappers to map the filetype keys to scintilla style enums? Obviously. My point was that selectively applying parts of a Scintilla update is acceptable if it is unlikely to cause bugs outside of use of the new feature being applied. You seemed to imply that it was dangerous/difficult to do that without updating all of Scintilla. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Adding some Glade widgets
On 09/01/2012 00:56, Matthew Brush wrote: - GeanyWrapLabel - Nice little existing GtkWidget that fixes broken GtkLabel text wrapping in GTK+ 2. Should work fine in a glade catalog and should even fix wrapping of labels in Glade. This would be really nice to have, I hate having to create wrapping labels manually whilst the other nearby labels can be done in Glade. - GeanyObject, others? Why would we need GeanyObject from Glade? It's a singleton. - GeanyScintilla - Subclass of ScintillaObject, all functions in sciwrappers moved to methods of this class. In future, could be combined There was a GtkScintilla project IIRC that did this, but I don't think it's maintained now. with GeanyEditor since both are somewhat redundant. I think keeping the separation of scintilla utils and editor functions is a good idea (e.g. derive GeanyScintilla from a GtkScintilla class). Anyway, that would be extremely disruptive to the plugin API. Regards, Nick ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Weird Segfault Crash
On 02/01/2012 14:33, Colomban Wendling wrote: encodings_convert_to_utf8_from_charset(utf8_name, (gsize) -1, ...) Is it OK the cast a negative number to `gsize` and will it have the desired effect for that function? The `-1` here is supposed to tell the encoding function that the string is nul terminated and is to be measured with `strlen()` IIUC. It's ugly, but AFAIK it'll work fine everywhere since it's casted back to gssize in that function. But you're right, it should definitely be a gssize parameter. Unfortunately this function is in the plugin API, and I'm not sure of the implications of changing that parameter's type... That should be fixed. I'm not sure whether it needs an ABI break or not (guess not), but if the parameter can be -1 then make it gssize. Nick ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
[Geany-devel] Glade 3 version?
Hi, I tried opening data/geany.glade with the latest Glade, 3.8.1 on Windows. Pressing Save writes a lot of changes to the file, 260 Kb. It seems to be mostly reordering property tag lines for use_action_appearance. Also, when I added 2 menu items it created duplicate image ids for image1, image2. What version of Glade created the file? Maybe if I can use the same version I won't get the reordering noise in the diff. Regards, Nick ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Glade 3 version?
On 19/12/2011 14:40, Matthew Brush wrote: On 12/19/2011 05:54 AM, Nick Treleaven wrote: Hi, I tried opening data/geany.glade with the latest Glade, 3.8.1 on Windows. Pressing Save writes a lot of changes to the file, 260 Kb. It seems to be mostly reordering property tag lines for use_action_appearance. Also, when I added 2 menu items it created duplicate image ids for image1, image2. Not sure what this means. It stopped Geany from starting with an error message about image1 being defined twice. What version of Glade created the file? Maybe if I can use the same version I won't get the reordering noise in the diff. I can't remember 100% but I think it was either 3.6.x or 3.8.x. I did however just make this[1] change with 3.8.0 for sure recently. Ok, there doesn't seem to be a 3.8.0 binary for Windows. The 3.6 binary had issues on my old machine so I'd rather stay on 3.8.1. It includes a relevant fix: - Ensure 'use-action-appearance' is serialized before 'related-action' (bug 658497) This is probably the cause of 99% of the diff. IMO, if you can't get the changes down to less, it doesn't really matter if there's a bunch of noise in the commit, it's not like anyone really needs to be able to read it, as long as the commit message describes I disagree no one needs to read it. glade diffs should be reviewed by the author the same as all other code checkins IMO. I'm hoping that we can standardise on 3.8.1 so we can review diffs and also avoid adding noise/bloat to the git repo each time someone uses a different version of Glade than the last commit. Before we decided to standardise on a glade version for 2.x to prevent the same problem (although this was also because 2.x didn't have a required gtk option, different versions added a lot of diff noise anyway). what was changed and it's not just Windows munging the file (with \r\n, etc.). I always choose the default core.autocrlf(?) option for msys-git as recommended by github, and the diff was produced by git. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Stub project files for sharing
On 08/11/2011 21:11, Lex Trotman wrote: [...] Unfortunately this is a requirement for any overriding scheme like stubs, if all the settings are written to the users project file no settings will be read from the stub file. No, what I would do is: * Read the user config file content * Append the content of the VCS file * Pass the data into a GKeyFile The key file will let the later entries naturally override the earlier user settings. This means we hardly have to change any code. The no write unless changed rule only applies to settings that can go in the stub file of course. Build settings already adhere to this rule. Of course splitting the project file does not have this problem, but as you point out has other problems. Build settings are unique ;-) IMO it's unjustified to change all project settings in core and plugins, plus require all future settings to follow the write-if-changed rule, which needs extra code per setting. Just to note, going the other way and having the stub file override the user settings is likely to make the code harder and the user interface more confused. Since we agree that the stub file isn't written in normal operation, users (and plugins) will need to be prevented from changing settings which are overridden by the stub, otherwise the changes will be written to the user file and overridden again by the stub when next opened. Having changes silently disappear is bad design. On saving I think you could detect a conflicting setting by comparing the keyfile values as strings, so warning the user they have made a change (and which one) that will be overriden. That is good enough IMO. [...] I think you might have missed that there is a contradiction between the statement above, tell the user but don't do anything to fix it, and the statement below (which I agree with) that they must be able to save changes to settings. That means user project settings must be loaded after the base project settings, the opposite of what you said above. But if user settings override base settings and all settings are written to the user file then no base settings will ever be used, so only changed settings should be saved in the user file. I think you overstate the amount of work, the project file doesn't hold much outside build (which does it), and session (which doesn't matter). The simple solution is to add only save if changed as a new feature in stash so its easy to use for the settings that need it. There is no contradiction - the VCS file overrides the user settings (which are the same as now) as I have explained. You detect conflicts and warn the user with the keyfile keyname. Detection is done just before saving the user project file - the user settings are stored in a GKeyFile - then the VCS file is loaded as a keyfile struct also. For each key in the VCS keyfile you read the corresponding string value of the key and compare it with a lookup in the prepared user keyfile. If there is a conflict, tell the user, and you could even prevent the prepared keyfile being saved to disk (but I'm undecided about that). The code I've described stays the same irrespective of which settings are in either keyfile, and doesn't require modifying existing code, just adds stuff. I don't mind if you don't like this solution, but it is probably the simplest in terms of code changed added. Hold on, build commands should be overridable by the user. The VCS file can provide initial commands, but the user should definitely be able to override them IMO. BTW you may want to disallow overriding build commands for some projects, this is Ok. I just think that at least run commands often need to be overridable. Also, I don't understand why using a visual merge program like Meld/WinMerge couldn't be used to update the VCS project file from a user file. This would make updating the VCS file quite easy IMO. And remember that probably only a minority of Geany users would be using shared project files, so I think the code should be minimal. If people disagree, write a plugin. Plugins can't set build settings since you removed the build plugin interface ;D If there was a general build API a plugin would not be able to use it in the way you imagine, it would need extra API elements to do so. To summarise, no suitable solution has been proposed so far: 1. splitting the files has backward compatibility unresolved so far 2. stub/base proposal has issues of overriding order vs save only changed unresolved so far I don't think (2) is true, it may have other limitations which personally I think don't matter, but it would work and be simple. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Short function factoring
On 08/11/2011 21:21, Lex Trotman wrote: On Wed, Nov 9, 2011 at 3:40 AM, Nick Treleaven nick.trelea...@btinternet.com wrote: On 04/11/2011 00:21, Lex Trotman wrote: * long functions cause bugs Rubbish, wrong functions cause bugs no matter how long, and being unable to see the whole logic leads to wrong design. Long simple functions are Ok. Obscuring the logic is obviously bad. Sub-functions should be near the function that uses it if the sub-function is only used once. * too many variables in one place cause bugs Thats what declarations in inner blocks are for :) That helps a bit, but the more things you have to keep track of the more likely you'll forget to initialize a variable that needs it or forget to free memory or other bugs. I'm sure there are statisics to back this up, it's well known in code analysis/reliability circles. And there are also statistics that say the opposite, that breaking it up too far over complicates things and causes unreliability. Agree, I should have included some numbers in my points. It depends on context, but I would say a function longer than 15 lines may be too long. More than about 7 variables can get hard to keep track of everything. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Short function factoring
On 09/11/2011 02:35, Matthew Brush wrote: On 11/08/2011 01:21 PM, Lex Trotman wrote: On Wed, Nov 9, 2011 at 3:40 AM, Nick Treleaven nick.trelea...@btinternet.com wrote: Breaking up logical tasks into functions is crucial to writing maintainable code, functions *are not just about code reuse*. But there's lots of places in Geany's code where logical tasks are actually further broken into many little sub-tasks (see below for an example), so small and trivial that the code would be a lot easier to follow if it was just in one place and achieved it's primary task. I didn't mean to imply Geany's code is perfect, or that I usually write perfect code. I just wanted to draw attention to what I think the ideal is. Obviously I didn't explain it very well. I guess we won't ever all agree on this, so we all have to be tolerant. Sadly, probably not :( Actually, I support the removal of functions which make code harder to follow, that is my only goal in discussing code style anyway. E.g. I said I agreed about the removal of editor_lexer_get_type_keyword_idx(). ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Proposal: Project settings split - adding geany features
On 04/11/2011 00:21, Lex Trotman wrote: Just one note that the point of raising it in the ML before doing it was to see if anyone had better ideas for a solution. Geany needs more of that, as exhausting as it may be, rather than taking the first suggestion or patch or pre-implemented pull request. So thanks to Nick and all who contributed. Agree if people are likely to disagree on it, if the feature changes much code or causes implications for future additions etc. (Obviously we don't want to discuss everything as it would cause too much delay). My points are not just dev issues - users may want to backup their project file, which would be harder. I don't understand how it would be harder, but anyway thats a specific. You would have to backup 2 files instead of 1 - not hard, *if* you understand this, but still more bother. Users need to read the manual, which would take longer to understand. Again this is a specific, but I don't understand how having session info in a session file and project info in a project file is harder to understand, or that most users would even need to care. I didn't say harder, I said it would take longer. Anyway sometimes they do need to know (e.g. backup). Users might have to use buggier software, as more complex code is always more bug prone. Development issues become user issues indirectly. On 04/11/2011 00:21, Lex Trotman wrote: Complicated isn't an absolute, it very much depends on experience, something you think is blindingly obvious I may find horrendously complex because I've never thought of that paradigm before. I agree that if the implementer considers it simple it is more likely to be right, but that doesn't guarantee that all maintainers will easily get their heads around it. Complicated means more code changed than necessary (for sake of argument). ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
[Geany-devel] Short function factoring
On 04/11/2011 00:21, Lex Trotman wrote: Geany has many places where a short function then calls another short function which calls another short function, none of which are re-used. Personally I find this way of writing code less efficient and very hard to follow and understand as a whole, but others find it easier to think only in terms of each little piece. YMMV Here may be somewhere where we disagree fundamentally, because: * long functions cause bugs * too many variables in one place cause bugs I'm sure there are statisics to back this up, it's well known in code analysis/reliability circles. Breaking up logical tasks into functions is crucial to writing maintainable code, functions *are not just about code reuse*. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Stub project files for sharing
On 04/11/2011 21:31, Lex Trotman wrote: [...] I see that 'based on' could be useful for making subprojects, and could be added to my proposal. As I see it, this is the same as your proposal, just that we have an explicit pointer from the project file to the stub project file so they don't *have* to be in the same directory and we don't have to go searching for the stub to know if we need to open it, that can be slow on networked locations. Yes. Note that this requires that projects not write a setting unless it is set by the user, that is not always the case now IIRC and is an added complication. I don't think this is a good requirement. Plugins already can write settings so I think it will be hard to enforce. But also rewriting existing code can be avoided and would be better. Unfortunately this is a requirement for any overriding scheme like stubs, if all the settings are written to the users project file no settings will be read from the stub file. No, what I would do is: * Read the user config file content * Append the content of the VCS file * Pass the data into a GKeyFile The key file will let the later entries naturally override the earlier user settings. This means we hardly have to change any code. The no write unless changed rule only applies to settings that can go in the stub file of course. Build settings already adhere to this rule. Of course splitting the project file does not have this problem, but as you point out has other problems. Build settings are unique ;-) IMO it's unjustified to change all project settings in core and plugins, plus require all future settings to follow the write-if-changed rule, which needs extra code per setting. Just to note, going the other way and having the stub file override the user settings is likely to make the code harder and the user interface more confused. Since we agree that the stub file isn't written in normal operation, users (and plugins) will need to be prevented from changing settings which are overridden by the stub, otherwise the changes will be written to the user file and overridden again by the stub when next opened. Having changes silently disappear is bad design. On saving I think you could detect a conflicting setting by comparing the keyfile values as strings, so warning the user they have made a change (and which one) that will be overriden. That is good enough IMO. Note that doing that does not require code per-setting, which is what I think can be avoided to mean much less of an impact on our codebase. But I think the code to block the UI and plugins is likely to be even worse than the alternative of write only on change. Don't understand where this idea comes from, sorry. This way the VCS file is still a project file and can be edited using the Geany UI, albeit at the cost of writing session crap into it, but as that is overridden by the user session crap on next open so it doesn't matter. I know that using Geany to create the VCS project would be easier, but stub creation is something that needs to be done rarely. I don't think it's worth rewriting existing code to support something that gets used rarely - use of projects depending on a stub is something that might be a popular feature (I realized eventually ;-)), but the actual stub creation and modifications is probably rare. I think you are underestimating how often the project settings, particularly build commands, change in the early stages. For mature projects it would be rare as most by then would also be fully integrated into make or similar, but early stages commands change more as the system develops. If the stub file is still a project file then no extra code is needed. Hold on, build commands should be overridable by the user. The VCS file can provide initial commands, but the user should definitely be able to override them IMO. Also, I don't understand why using a visual merge program like Meld/WinMerge couldn't be used to update the VCS project file from a user file. This would make updating the VCS file quite easy IMO. And remember that probably only a minority of Geany users would be using shared project files, so I think the code should be minimal. If people disagree, write a plugin. I think having a separate filetype for the VCS file is good design because it shouldn't be written to, so it is different from a normal geany project file. Agree it shouldn't be written to in normal use. I don't see any point in writing code to save the contents in a different structure to the standard one, that adds code, makes it hard to hand edit since you have to translate settings from one structure to the other, you can't just copy an existing file etc. But I don't think that is actually what you meant. No. The contents would be a subset (user decidable for my proposal) of a normal project file. Just changing the extension would reduce accidental use, but even calling it fred.manglewurzle won't
Re: [Geany-devel] Stub project files for sharing
(There was no reply-to list address set for Thomas's mail (weird), so I'm now sending this to the list - possibly a Thunderbird issue). On 04/11/2011 09:56, Thomas Martitz wrote: Am 03.11.2011 15:45, schrieb Nick Treleaven: My solution: A foo.geanystub project file goes in version control. It is never written to. It should be prepared by manually editing a copy of a local project file. On opening a foo.geanystub file, Geany creates foo.geany in the same path then opens it. When opening foo.geany, if foo.geanystub exists, then override settings with the stub contents. This way the VC file can decide which settings are not overridable. My solution shouldn't require much code to implement. I've only noted the bare bones of it, there are some things that could be added to make it better. Even with these I think it's simpler and neater. That solution seems awkward to me. Especially the creation part where you need to copy, then hand-edit. Normally you don't need to touch such project files at all, but you require not only that, but also that the people know which parts to take out. It really isn't that difficult, and it only has to be done once, plus any minor modifications later. For something rarely done, Geany shouldn't need to alter its existing code too much - see my answer to Lex for more explanation. Supporting GUI creation is much more work for Geany. Also, one could argue that foo.geany should override the stub file. Anyway this is a feature not covered by the separate-file approach. This could be added to the stub approach (if we decide we want it). I don't think it could be added to Lex's proposal. So, it seems more complicated for users, since the separate-file approach would just work (no hand-editing required), so I disagree it's simpler. OTOH it's probably simpler to implement (just load the stub after, so things get overridden automagically) and backward-compatibility is not an issue. Also there is the problem with the original idea of deciding which settings go in each project file - there isn't always a right answer. Regards, Nick ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Stub project files for sharing - creation plugin
On 04/11/2011 15:22, Nick Treleaven wrote: That solution seems awkward to me. Especially the creation part where you need to copy, then hand-edit. Normally you don't need to touch such project files at all, but you require not only that, but also that the people know which parts to take out. It really isn't that difficult, and it only has to be done once, plus any minor modifications later. For something rarely done, Geany shouldn't need to alter its existing code too much - see my answer to Lex for more explanation. Supporting GUI creation is much more work for Geany. Also someone could write a plugin to create stub project files that supports the common settings that would go in a stub project file. That would be less effort as it would only need to support a fraction of project settings. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Make document_save_file() show the Save As dialog when necessary - autosave
On 02/11/2011 23:22, Lex Trotman wrote: Hi Nick, Good idea, I have one question below. [...] It's possible plugins may need updating, but after a quick grep it seems perhaps only the core Save Actions plugin. For auto-saving it may want to ignore documents without an absolute path to avoid showing the Save As dialog. I'll make this change next. Does this skip any documents that the user might reasonably expect to autosave? Obviously it skips new as yet unsaved files, but IMHO thats ok, is there anything else? Well, as documents that have been saved now have to have an absolute path, I don't think it would skip saving any document that exists on disk. Although I'd better check that opening documents enforces an absolute path too, I think so. The autosave check could actually be against doc-real_path, which is only set if the file exists on disk, rather than checking for an absolute path. If so we might want to indicate the fact that this file isn't being autosaved to warn that there is a risk of data loss. (maybe with new files too?) For modified documents the user can see this already, but for generated documents that don't set the modified flag data could be lost, but this probably tends to be things which the user isn't concerned about. E.g. with a geanyVC diff, you usually want to just close the document and ignore the data. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Proposal: Project settings split - adding geany features
On 02/11/2011 23:03, Lex Trotman wrote: [...] Everything else is shared via VCS, so its annoying that one thing isn't. As I said, you can already share settings, you just can't easily synchronize them *if* they change. Whats the problem with doing it? It adds code complexity, plus backwards compatibility would make the code more ugly. Having all project settings stored in the same file is neater and easier to understand for users. It makes the manual more complex, maybe helping a little to obscure the important stuff. Nick, this argument is saying that Geany should never change or improve, if you want to stagnate just keep using 0.21 or your favourite version ... Storing information with different uses (personal vs project) in the same file makes life harder for some users, all your arguments only relate to development issues not user issues. First, I'd like to address these points, but I have actually thought of an alternative proposal which hopefully you might like and has much less impact on code. I'll start a new thread. The point of arguing this out isn't especially for this feature, but for considering adding other features too. My points are not just dev issues - users may want to backup their project file, which would be harder. Users need to read the manual, which would take longer to understand. Users might have to use buggier software, as more complex code is always more bug prone. Development issues become user issues indirectly. I could bring up the Eclipse argument to say that worse is better. The more use cases Geany caters to, the more Eclipse-like it will be. If there are going to be restrictions on what changes are made to Geany in the future then you need to put that proposal to the Geany community to consider how such decisions are made. No, and also I'm not maintainer any more. But we do need to consider whether adding features is worth the maintenance and possible disruption. Perhaps I was wrong to bring up Eclipse in this case, but I do think there are issues with your proposal which make it bad - as they're related to my proposal I'll deal with them in the new thread. Regards, Nick ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
[Geany-devel] Stub project files for sharing
Hi, I think I can improve on Lex's recent proposal to split up project settings into two files, one for version control and one for local changes. The problems with that are: * How do we decide which settings are in each - e.g. build menu commands - a project might want to override the make command in the VC settings file, but the user will want to add their own build commands too. There's a conflict. This becomes increasingly messy as we add project settings. * Backwards compatibility - we would need to handle existing project files anyway, which makes code more complex. I don't think we need to break compatibility. * If the user changes a setting which is stored in the VC file they may end up committing it, or perhaps sometimes not even realizing they are overriding the VC setting. My solution: A foo.geanystub project file goes in version control. It is never written to. It should be prepared by manually editing a copy of a local project file. On opening a foo.geanystub file, Geany creates foo.geany in the same path then opens it. When opening foo.geany, if foo.geanystub exists, then override settings with the stub contents. This way the VC file can decide which settings are not overridable. My solution shouldn't require much code to implement. I've only noted the bare bones of it, there are some things that could be added to make it better. Even with these I think it's simpler and neater. Nick ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Proposal: Project settings split
On 01/11/2011 22:24, Lex Trotman wrote: On 2 November 2011 00:09, Nick Treleavennick.trelea...@btinternet.com wrote: On 31/10/2011 23:50, Lex Trotman wrote: IIUC the functionality would be the same but with different storage? If so I'm not sure this would be worthwhile complexity. A starting project file could be hand edited and put in version control, for people to copy into their projects directory, i.e. not use the VC file itself. That doesn't keep the settings up to date as they need to change, thats the purpose of keeping it in the VCS. Yes, I just question whether it's worth supporting that. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Proposal: Project settings split
On 02/11/2011 12:10, Lex Trotman wrote: [...] Yes, I just question whether it's worth supporting that. Well, it comes down to how groups and individuals work, you may not want to share projects settings, but I am part of groups who do. Everything else is shared via VCS, so its annoying that one thing isn't. As I said, you can already share settings, you just can't easily synchronize them *if* they change. Whats the problem with doing it? It adds code complexity, plus backwards compatibility would make the code more ugly. Having all project settings stored in the same file is neater and easier to understand for users. It makes the manual more complex, maybe helping a little to obscure the important stuff. I could bring up the Eclipse argument to say that worse is better. The more use cases Geany caters to, the more Eclipse-like it will be. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
[Geany-devel] Make document_save_file() show the Save As dialog when necessary
Hi all, I've just made a commit which changes plugin API behaviour, which I think is necessary. Please see the full log message below for justification. It's possible plugins may need updating, but after a quick grep it seems perhaps only the core Save Actions plugin. For auto-saving it may want to ignore documents without an absolute path to avoid showing the Save As dialog. I'll make this change next. Note that file templates and other features (geanyVC?) create documents with a filename but no path, hence this change. Remembering to show the Save As dialog in all code for the same reason was not reliable enough. That code can now be removed, but won't cause users any problems if left. On 02/11/2011 15:27, Nick Treleaven wrote: Branch: refs/heads/master Home: https://github.com/geany/geany Commit: de559ef5d4150e2485ef3ffb865e9c9d3249bcd8 https://github.com/geany/geany/commit/de559ef5d4150e2485ef3ffb865e9c9d3249bcd8 Author: Nick Treleavennick.trelea...@btinternet.com Date: 2011-11-02 (Wed, 02 Nov 2011) Changed paths: M src/build.c M src/callbacks.c M src/dialogs.c M src/document.c Log Message: --- Make document_save_file() show the Save As dialog when necessary Previously an error message was shown if doc-file_name is NULL. The Save As dialog is now shown if the document does not have an absolute path. This is because the user should confirm where to save the document in this case. Although this changes plugin API behaviour, it seems the best way to ensure the Save As dialog is always shown when needed so the user knows where the document has been saved. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Make document_save_file() show the Save As dialog when necessary
On 02/11/2011 15:44, Nick Treleaven wrote: Remembering to show the Save As dialog in all code for the same reason was not reliable enough. That code can now be removed, but won't cause users any problems if left. BTW the only plugin that seems to use dialogs_show_save_as() is geanysendmail - I'll make a patch to remove the now unnecessary code. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Proposal: Project settings split
On 31/10/2011 23:50, Lex Trotman wrote: Hi All, One area of Geany has been annoying me for some time. At the moment project settings mix session related settings and truely project settings. The session settings are really user specific whilst the project settings are project related. This means that if the project file is in VCS it keeps updating as the session settings change. You can turn session settings off, but then you lose that functionality. The proposal is to separate these, with the session settings stored in the user config directory under a projects subdir and the project settings stored wherever the user wants. The options ~/projects or in the tree would be available as they are now. IIUC the functionality would be the same but with different storage? If so I'm not sure this would be worthwhile complexity. A starting project file could be hand edited and put in version control, for people to copy into their projects directory, i.e. not use the VC file itself. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] GIT commit mails format
On 25/10/2011 00:33, Lex Trotman wrote: [...] If we go back to individual commit mails with diffs, is there anyway to limit how many individual messages get sent out at once and how many lines can be in a diff? Ex. if we merge the gtkbuilder branch at some point, is there anyway to prevent from flooding the list with all those commit mails but to send individual ones for regular 2 or 3 commits in a row? I'm not sure we would see a flood on a merge, probably only one commit saying 'merged gtkbuilder branch'? The gtkbuilder commits should appear as they are committed, not on merge. But really, I don't care too much either way :) And the flooding each time geany.html gets updated is a pain. This may have been because I had an older version of python/docutils. When I regenerated the html 3 weeks ago with a recent docutils the diff was small. Personally I don't mind the links and combined emails so I guess I'm -0.5. I'm +1, assuming we have diff size limiting like we used to with SVN. I find it very useful to look at small changes from my mail reader without having to click and wait for the github diff page to load - this discourages me from bothering. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] GIT commit mails format
On 31/10/2011 13:57, Matthew Brush wrote: I'm not sure we would see a flood on a merge, probably only one commit saying 'merged gtkbuilder branch'? The gtkbuilder commits should appear as they are committed, not on merge. I think when you merge it makes all the commits onto the branch that was merged into also. At least this is what happened on the xfce-commits list when one of the devs merged several branches with a total of 800+ commits in them, each triggering a commit mail :) Ok, but ideally it would do as I described, like the github commit list for the scintilla-update merge: https://github.com/geany/geany/commits Isn't --no-ff or --squash not able to prevent this? IMO, it's not good to mess with the history just to avoid triggering commit mails. Agreed. Besides I would forget to pass those options ;-) ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] GIT commit mails format
On 31/10/2011 15:07, Nick Treleaven wrote: On 31/10/2011 13:57, Matthew Brush wrote: I'm not sure we would see a flood on a merge, probably only one commit saying 'merged gtkbuilder branch'? The gtkbuilder commits should appear as they are committed, not on merge. I think when you merge it makes all the commits onto the branch that was merged into also. At least this is what happened on the xfce-commits list when one of the devs merged several branches with a total of 800+ commits in them, each triggering a commit mail :) Ok, but ideally it would do as I described, like the github commit list for the scintilla-update merge: https://github.com/geany/geany/commits Anyway I think the problem of only showing the first commit of the push in the subject makes the commit mails harmful because changes can easily be missed. This problem seems more important than the potential flooding issue. In practice our branch merges likely wouldn't have 800 commits. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] gtkbuilder branch and ui_lookup_widget functions
On 19/10/2011 08:47, Matthew Brush wrote: On 11-10-17 05:22 AM, Nick Treleaven wrote: Hi Matthew, I'm a bit concerned about the changed ui_lookup_widget (and hookup) functions - these are in the plugin API and can be used independently from Glade. (plugin) API function behaviour should not normally change, but it seems the owner parameter is now ignored. If we are caching lookups this is probably wrong as the widget may get destroyed. OK! I got this more or less fixed up and working like it used to. The one exception is some items from the editor menu and maybe a few other menu items. I think it's related to some hackery somewhere in the existing code that moving around menu items to share the same widgets in more than one menu. IMO it is necessary because there are often popup menu submenus that need to be identical, and maintaining duplicate menu items with Glade is a pain. If your code does what Glade 2's interface.c did then I'm not sure why there would be incompatibilites. I'm hoping someone more familiar with this part of the code can help me out here. You can see the changes here: https://github.com/codebrainz/geany/commits/gtkbuilder2/ There's still a few FIXME's and it needs to be cleaned up, but if you don't mind having a look to see if overall this is fixing the issues you raised. Could you move that branch into the geany repo? It would be easier to look at. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] [RFC] Geany Plugin Names / Lua Scripting
On 29/10/2011 06:41, Matthew Brush wrote: Hi all, Is anybody opposed to removing the geany and Geany prefix from the plugins in Geany-Plugins. I mean at least for the directory name in the source tree, README/Site, and PLUGIN_SET_INFO() name? It's always bothered me when I go into *Geany's* plugin manager and see plugin names like GeanyFooBar. Obviously the plugin is for Geany and the Geany prefix makes the plugin not ordered correctly. Same goes for the SVN/Git repository source tree directory names. +1 I think we should definitely drop Geany from all the names shown in the plugin manager, even for independently maintained plugins. GeanyLua Lua Plugins lua IIRC the PM name is 'Lua Scripting'. It doesn't really have a maintainer ATM but I am the named maintainer. Feel free to change 'GeanyLua' and/or the directory name - I would suggest 'luascript'. It doesn't allow true Lua plugins. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
[Geany-devel] project build tab - Re: GTK+ Version Bump to 2.18 - gtkbuilder and 2.16
On 18/10/2011 00:36, Matthew Brush wrote: On 11-10-17 04:27 PM, Lex Trotman wrote: On 18 October 2011 10:17, Matthew Brushmbr...@codebrainz.ca wrote: I also noticed some issues with defaults in the project dialog where some stuff doesn't have a default and some of the combo boxes have two columns showing so it looks like: Item 1 Item 1 Item 2 Item 2 IMO, it would *really* be nice to get all of the Project dialog ported to GtkBuilder since currently only the Editor and Indentation tabs are in GtkBuilder, the Project tab and the Build tab are both hard-coded and added to the dialog at runtime it seems. Hi Matthew, Yes, the build tab is generated at runtime because it has variable length (set by the number_xx_menu_items prefs). Possibly it could be a treeview, but that would change the whole operation of the build tabs and would need discussion with Nick who has strong ideas on the subject :) Not really, I'm just opposed to replacing groups of commands with a list of commands as I think this is worse usability. I think a treeview for each command group wouldn't really change things and might look better. Is there a way to show a label with mnemonic in a treeview column? Not crucial but nice to have. +1. I actually started tinkering with making it a treeview in Glade, since it makes way more sense to not have a hardcoded number and types of build commands, but of course as we discussed before, there needs to be a fixed number of commands at startup due to the limitations in the keybinding system. Not necessarily, but sort of. Keybindings that map to a non existent command could just do nothing. Anyway, for now, we could add the whole UI into Glade except the dynamic part and just append/insert rows for more commands at run-time for the parts that change. At least 95% of it would go into the UI file. Sounds good, but maybe it's best to wait until after the merge into master. I think branches should stay focused on one thing; that sounds like a bigger change. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
[Geany-devel] gtkbuilder branch and ui_lookup_widget functions
Hi Matthew, I'm a bit concerned about the changed ui_lookup_widget (and hookup) functions - these are in the plugin API and can be used independently from Glade. (plugin) API function behaviour should not normally change, but it seems the owner parameter is now ignored. If we are caching lookups this is probably wrong as the widget may get destroyed. I haven't looked at the new implementation but perhaps you could help explain the changes. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Geany memory behavior suspected memory leak - ID: 3415254 - g_slice
On 15/10/2011 00:08, Lex Trotman wrote: 1. gslice is good when you allocate lots of things the same size, and you know the size, since that must be supplied to slice_free, but a large portion of our allocations are strings, so we don't remember the size (see for eg tagmanager). Yes, by fixed size alloc I meant fixed size types, not strings. Secondly, see: http://developer.gnome.org/glib/2.28/glib-Memory-Slices.html#glib-Memory-Slices.description This is accompanied by extra caching logic to keep freed memory around for some time before returning it to the system. Yes, it waits 15 seconds before freeing empty slabs back to the system, thats what I mean by each of which has differing policies for holding onto memory. This is actually observable, but it only frees about 100k of the 28Mb. malloc and C++ work differently. Thirdly, GLib recommends using g_slice for fixed size allocations, and IIUC g_slice completely prevents fragmentation, besides being faster (anecdotally about 2x). No, even the glib developers don't claim it completely prevents fragmentation :) It completely prevents *internal* fragmentation Agreed. within a slab since all chunks are the same size, but no slab can be returned to the system or re-used for another size if it has any chunks still in use. And this form of fragmentation is very likely since Geany and GTK and tagmanager etc all hold on to random bits of memory (because they need to, its not a bug), thus blocking release of slabs. Yep, this is a problem. I don't know if converting our other uses of g_new for fixed size structures is worth it, but I would say it is good practice to use g_slice in all new code. Agree, I don't think we are going to be able to totally cure the problem but we can waste lots of effort on it. Actually it probably depends on the context as to whether g_slice is a good idea. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
[Geany-devel] geanydocument caching - Re: Geany memory behavior suspected memory leak - ID: 3415254
On 14/10/2011 19:26, Colomban Wendling wrote: 1. each time all files are closed only 200k is returned to the OS, but since the first open uses 22.8Mb but later ones less, Geany isn't leaking all of it, it is being held by the allocators. Geany at least keeps all the GeanyDocuments alive and tries to re-use them (document_create() at line 564). This avoids having to re-allocate the document struct everytime, though it'll then never release this memory. However, either the user won't open so much files at once or she's likely to keep having a large amount of open files, so I don't think it's a (the?) problem -- and anyway I guess the allocation would be quite small. Exactly, 100 GeanyDocument is still small. Also we don't reuse the private document structures. The reason for caching geanydocument structs is not efficiency, but so we can inspect doc-is_valid even after a document is closed. In hindsight I don't think this was really a good idea, but it came about mainly for backward compatibility with existing code (originally the document array was a fixed size). At the time we could have fixed the code that relied on geanydocuments never being freed, but now I think it might be too late. However, we'd probably better use GSlice here (for fragmentation and maybe caching), and maybe even release the memory to GSlice so it does all the management, and could even release it to the system at some point if it makes sense to it. If we didn't need to keep geanydocuments around, I think g_slice would be Ok for this. As it is we could change the geanydocumentprivate struct to use it, but it might not help much. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] GRegex - Re: editing big files can be too slow with tag reparsing
On 17/10/2011 18:18, Colomban Wendling wrote: GRegex is more reliable besides being more powerful, based on PCRE. The Windows version (at least) of GNU regex we had didn't understand Mac line endings for ^, $. Hehe, even better:) Now we should probably port all our regexes usage to GRegex -- and drop the included one when done, but there's no hurry. Yes, I added a TODO to this effect ;-) I'm planning on porting the find replace regex code sometime, unless someone beats me to it. The increased power for users should be nice. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Geany memory behavior suspected memory leak - ID: 3415254 - g_slice
On 14/10/2011 04:09, Lex Trotman wrote: Hi All, I have been investigating bug report suspected memory leak - ID: 3415254 http://sourceforge.net/tracker/?func=detailatid=787791aid=3415254group_id=153444 1. each time all files are closed only 200k is returned to the OS, but since the first open uses 22.8Mb but later ones less, Geany isn't leaking all of it, it is being held by the allocators. 2. ignoring the first open, the amount of extra memory used to open all the files varied from 0.4Mb to 2.9Mb. Average 1.1Mb or 5% of the initial 22.8Mb and standard deviation of 0.665Mb or 3% of the 22.8Mb. 3. Simple leaks are unlikely to cause the large deviation in the memory increase, each open of the same set of files would tend to leak the same amount. It is therefore likely that fragmentation effects cause the large deviation, but it may not be the whole cause of the increase. Since Geany uses three different allocators, each of which has differing policies for holding onto memory, it is going to be difficult to separate real leaks from allocator effects. Interesting. I'd like to continue the discussion from the report about allocator usage: Lex: I think probably most of the fragmentation is due to having three allocators in use, libc, Glib and libc++, and they not sharing or returning memory they acquire. This is likely to outweigh the cost/benefit of switching to g_slice given the effort required since types or sizes are needed in all frees. Firstly, we already are using g_slice quite a lot - probably the most useful place is for allocating TMTag structures. Obviously GLib uses it internally. Secondly, see: http://developer.gnome.org/glib/2.28/glib-Memory-Slices.html#glib-Memory-Slices.description This is accompanied by extra caching logic to keep freed memory around for some time before returning it to the system. Thirdly, GLib recommends using g_slice for fixed size allocations, and IIUC g_slice completely prevents fragmentation, besides being faster (anecdotally about 2x). I don't know if converting our other uses of g_new for fixed size structures is worth it, but I would say it is good practice to use g_slice in all new code. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] GTK+ Version Bump to 2.18 - gtkbuilder and 2.16
On 13/10/2011 12:53, Nick Treleaven wrote: On 13/10/2011 00:55, Matthew Brush wrote: So I would go for 2.16 overall if this brings us Glade 3 support. It should, but I haven't thoroughly tested it with GTK+ 2.16 yet. Did you try the gtkbuilder branch yet on your 2.16 install by any chance? No, I haven't tried git branching yet. I'll let you know when I do. Tried it earlier, seems to build against 2.16 (I actually have a newer runtime) and run OK after I applied some fixes. I didn't really test it much besides showing the Prefs dialog. Anything in particular that needs testing? I did get a warning: Gtk WARNING : Could not find signal handler 'on_term_font_set' ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
[Geany-devel] per-project prefs - Re: Remove extra whitespace at end of lines in all source files?
On 13/10/2011 22:24, Matthew Brush wrote: On 10/13/2011 04:42 AM, Nick Treleaven wrote: The stripping feature in Geany should be made a project preference, I don't want to cause noise on non-Geany files. This is actually on my TODO list, unless anyone feels like doing it before me :) Great. BTW the long line marker project setting has an option 'Use global settings'. I don't think we should repeat this for all new settings as it doesn't scale well. Perhaps there could be one 'Override global settings' per tab, but this isn't important. Also, presumably we should avoid making any glade changes on master until after merging the gtkbuilder branch? ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] GTK+ Version Bump to 2.18 - gtkbuilder and 2.16 - on_term_font_set
On 14/10/2011 15:38, Nick Treleaven wrote: I did get a warning: Gtk WARNING: Could not find signal handler 'on_term_font_set' This is because on_term_font_set is not defined unless VTE support is compiled in (on Windows it's disabled). ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Remove extra whitespace at end of lines in all source files?
On 12/10/2011 23:50, Matthew Brush wrote: On 11-10-12 07:14 AM, Nick Treleaven wrote: On 12/10/2011 06:00, Matthew Brush wrote: Remove extra whitespace at end of lines in all source files. * Processed with rstrip-whitespace.py script added to scripts/ directory. * Script run on all .c and .h files in src/ and plugins/ directories. * Also remove more than one newline at the end of files. We've mostly done this before, but I'm not sure this is a good idea. It means more maintenance burden on receiving code. What's so bad about trailing whitespace? The main reasons are to avoid noise in patches, prevent patch corruption Unfortunately it can cause noise also. when sent over email, and IMO it's just good practice (ie. what people contributing code would expect). If we don't care about this, then why care about spaces vs. tabs, spaces before function parameter braces, indentation, and so on? Those things affect readability. Also, I'm not suggesting that we reject code solely because of trailing whitespace, just that it become something the developers do; I would guess that the vast majority of what I removed in that commit was caused by Geany developers themselves. It takes two seconds to turn the stripping feature on in Geany or Git How do I turn it on for Git? The stripping feature in Geany should be made a project preference, I don't want to cause noise on non-Geany files. and it automatically handles the bug in Geany's auto-indent where it adds the indentation when you press enter but doesn't remove it when you leave the line blank. This is where the bulk of the trailing whitespace came from. As of for the extra newlines at the end of files, I think those are caused by a bug in one of the processing scripts used on Geany's source code (fix-alignment.pl:91 maybe). The extra newlines were probably added by my function snippet that includes 2 trailing newlines. If you really feel strongly against this, I honestly don't mind if you revert the commit and we can just forget about it altogeher. FWIW, I did check with Colomban about this, confirming that there shouldn't be trailing whitespace and that it was ok to commit this, before doing so. Sorry for such a long response to something so trivial :) I just thought I'd raise it. If we have Git support for this then it's not much of an issue. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] GTK+ Version Bump to 2.18
On 13/10/2011 00:55, Matthew Brush wrote:
On 11-10-12 07:04 AM, Nick Treleaven wrote:
On 12/10/2011 02:58, Matthew Brush wrote:
I've also been working on getting rid of some of the uses of sealed
members (ex. widget-window as opposed to
gtk_widget_get_window(widget)). It seems many of the accessor functions
were added between 2.12 and 2.18. One of the functions that comes to
mind that needs 2.18 and could be used in several places in Geany is
gtk_widget_get_allocation().
OTOH, those functions are trivial to support on 2.18, if there were a
reason to prefer lower versions. If there's not too many.
There's lots :) [1]
[1]
https://github.com/codebrainz/geany/commit/06e27060fd5022d22160d51ca62c3601f649f28f
That includes changes that work on GTK 2.0 - what I meant is how many
functions are added in 2.18 that aren't in 2.16 that could be included with:
#if gtk 2.18
bar gtk_foo_get_bar(){return foo-bar;}
#endif
I didn't look at the diff much because it didn't really answer this.
So I was wondering if anyone was opposed to going from GTK+ 2.12 to 2.18
as the minimum supported GTK+ version. IMO, if we are going to raise the
version this release cycle, it makes sense to do so sooner rather than
later to maximize the time for finding and fixing bugs and so on.
From my own POV I would prefer lower versions for now - I have 2.16 on
my main machine. I could update but I'm still reeling a little from the
github switch ;-)
So the answer to my question in the other thread about what specifically
sets the minimum GTK+ version Geany supports is: Whatever Nick is
running :)
Not necessarily, I could upgrade as I'm on Windows, but for now I'd
prefer not to.
A more important issue is maximising who can contribute to Geany, so
it's good to not require anything released in e.g. the last two years
without good reason. I know 2.16 is probably before that though.
So I would go for 2.16 overall if this brings us Glade 3 support.
It should, but I haven't thoroughly tested it with GTK+ 2.16 yet. Did
you try the gtkbuilder branch yet on your 2.16 install by any chance?
No, I haven't tried git branching yet. I'll let you know when I do.
Anyway, I won't push for GTK+ 2.18 any more if some core Geany devs are
against it.
I'm not completely against it but just want to make sure it brings real
benefits for us.
___
Geany-devel mailing list
Geany-devel@uvena.de
https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
[Geany-devel] Remove extra whitespace at end of lines in all source files?
On 12/10/2011 06:00, Matthew Brush wrote: Remove extra whitespace at end of lines in all source files. * Processed with rstrip-whitespace.py script added to scripts/ directory. * Script run on all .c and .h files in src/ and plugins/ directories. * Also remove more than one newline at the end of files. We've mostly done this before, but I'm not sure this is a good idea. It means more maintenance burden on receiving code. What's so bad about trailing whitespace? ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Geany is on Github
On 07/10/2011 16:28, Matthew Brush wrote: Just to start, because I had their Github usernames, I've added Enrico and Colomban as owners, as well as myself since I need access to set the stuff up. Whoever else should be an Owner (Frank, Nick, etc.) just let me (or other Owners) know. Read below as for why not too many people really need to be Owners. I've now created an account, ntrel. And now I have a question; now that Geany and Geany Plugins are grouped together under the same organization, should we drop the geany- prefix on the plugins repository? I would keep geany-plugins to show it's the g-p release, e.g. not core plugins. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Geany is on Github
On 07/10/2011 17:55, Thomas Martitz wrote: Now, what we could do, for example, is to create another Team called Contributors or something, and this Team could have Pull only access (even without being in a team, anyone can still clone/pull). Myself, I would probably go into the Contributors Team, since I contribute a fair bit, but I don't have commit access. Man, give this man commit access already! You've done so much invaluable work for this project, you well deserved it. I agree, I support Matthew joining the core team. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] CTags with GRegex - Re: editing big files can be too slow with tag reparsing
On 02/10/2011 20:16, Jiří Techet wrote: On Sun, Oct 2, 2011 at 16:34, Nick Treleaven Now done. This fixes my performance issue with doc/geany.html. Needs testing though. Regex parsers are: actionscript cobol html php R Link: http://geany.svn.sourceforge.net/viewvc/geany?view=revisionrevision=5976 I think Colomban's idea of using GRegex is great in that all systems will then have the same regex behaviour. The old situation was a support headache. GRegex is more reliable besides being more powerful, based on PCRE. The Windows version (at least) of GNU regex we had didn't understand Mac line endings for ^, $. Cool! By the way, are the #ifdef HAVE_REGEX needed? From glib sources it seems GRegex either uses system pcre or built-in pcre so you should have always regex support. I left HAVE_REGEX in lregex.c so less code is different from CTags. I think it doesn't cause any harm. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] CTags with GRegex - Re: editing big files can be too slow with tag reparsing
On 30/09/2011 17:32, Nick Treleaven wrote: On 30/09/2011 14:37, Nick Treleaven wrote: On 27/09/2011 13:17, Nick Treleaven wrote: On 26/09/2011 23:00, Colomban Wendling wrote: Le 26/09/2011 13:48, Nick Treleaven a écrit : I guess the reason is the regex code that is really old which is used for Windows builds. Maybe we could use GRegex now we have a GLib recent enough? I'm now working on porting lregex.c to GRegex, I think it might be compatible enough. I'll commit it to unstable if it goes OK. Now done. This fixes my performance issue with doc/geany.html. Needs testing though. Regex parsers are: actionscript cobol html php R Link: http://geany.svn.sourceforge.net/viewvc/geany?view=revisionrevision=5976 I think Colomban's idea of using GRegex is great in that all systems will then have the same regex behaviour. The old situation was a support headache. GRegex is more reliable besides being more powerful, based on PCRE. The Windows version (at least) of GNU regex we had didn't understand Mac line endings for ^, $. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Breaks not fixes : SF.net SVN: geany:[5971] trunk/src/socket.c
On 30/09/2011 02:32, Lex Trotman wrote: Modified: trunk/src/socket.c === --- trunk/src/socket.c 2011-09-29 17:16:58 UTC (rev 5970) +++ trunk/src/socket.c 2011-09-29 17:49:42 UTC (rev 5971) @@ -588,7 +588,7 @@ gint fd, sock; gchar buf[BUFFER_LENGTH]; struct sockaddr_in caddr; - guint caddr_len = sizeof(caddr); + gint caddr_len = sizeof(caddr); GtkWidget *window = data; gboolean popup = FALSE; Hi Nick, This causes a warning on the call to accept() on Linux. CC socket.o socket.c: In function ‘socket_lock_input_cb’: socket.c:596: warning: pointer targets in passing argument 3 of ‘accept’ differ in signedness /usr/include/sys/socket.h:214: note: expected ‘socklen_t * __restrict__’ but argument is of type ‘gint *’ caddr_len s/b a socklen_t not a ganything, that works here. Hopefully on your system it will be able to be assigned from sizeof without a warning. Looks like your crappy windows system :) has sizeof signed, but on most Linux systems its unsigned. No, the assignment wasn't the warning, just the pointer signedness. If socklen_t doesn't match sizeof then special windows handling is going to be needed. I misunderstood a note on the linux manpage for accept, thinking int is the proper type. Thanks for finding the bug. I've reverted the change for the 0.21 release, just in case some systems don't have socklen_t, but used socklen_t in the unstable branch. I should have wondered why I hadn't seen the warning on Linux. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
[Geany-devel] CTags with GRegex - Re: editing big files can be too slow with tag reparsing
On 27/09/2011 13:17, Nick Treleaven wrote: On 26/09/2011 23:00, Colomban Wendling wrote: Le 26/09/2011 13:48, Nick Treleaven a écrit : I'm reduced to using Windows. I guess the reason is the regex code that is really old which is used for Windows builds. Maybe we could use GRegex now we have a GLib recent enough? I don't know if GRegex would work, CTags parsers may use non-POSIX features - are \w \s part of POSIX? I'm now working on porting lregex.c to GRegex, I think it might be compatible enough. I'll commit it to unstable if it goes OK. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] CTags with GRegex - Re: editing big files can be too slow with tag reparsing
On 30/09/2011 14:37, Nick Treleaven wrote: On 27/09/2011 13:17, Nick Treleaven wrote: On 26/09/2011 23:00, Colomban Wendling wrote: Le 26/09/2011 13:48, Nick Treleaven a écrit : I'm reduced to using Windows. I guess the reason is the regex code that is really old which is used for Windows builds. Maybe we could use GRegex now we have a GLib recent enough? I don't know if GRegex would work, CTags parsers may use non-POSIX features - are \w \s part of POSIX? Actually I was wrong, our parsers don't use \w or \s. I'm now working on porting lregex.c to GRegex, I think it might be compatible enough. I'll commit it to unstable if it goes OK. Now done. This fixes my performance issue with doc/geany.html. Needs testing though. Regex parsers are: actionscript cobol html php R ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Bug on utils_string_replace()?
On 28/09/2011 20:56, Colomban Wendling wrote: Le 28/09/2011 19:36, Frank Lanitz a écrit : On Sun, 25 Sep 2011 20:11:06 +0200 [...] Just want to bring it up again. Release is near so at least we should add a comment to api documentation about this. Right. Fixed now in r5965, thanks. If we want to deprecate the use of NULL here, I think we better do it after the release so plugin developers have a chance to fix their use until next-next release. Thanks for fixing my mistake. I think there's no need to stop accepting null on utils_string_replace_all. It's bad to change plugin API behaviour unless the old use will get caught by the compiler. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] editing big files can be too slow with tag reparsing
On 28/09/2011 00:45, Lex Trotman wrote: Nick, Thomas, [...] 0001-Per-document-real-time-symbols-setting.patch: Adds a per-document setting for real-time updates and a menu item in the Document menu. There is still a FIXME in it, feel free to give ideas :) @Nick, there are lots of large generated files a programmer might want to check, especially XMLs and logs, saying there is an xxxGb maximum is wrong, it depends on the machine the user has. Forcing them to have to go use another editor or to not have highlighting is bad. You edited out my reply which addressed that: On 27/09/2011 17:17, Nick Treleaven wrote: 1. Very big files x MB - I don't think we need to support that case specially, Geany is a programmer's editor. The user can just open as filetype none if they only want basic editing. In the open dialog, choose filetype None and you can easily open big files, plus it will be faster as syntax highlighting won't be enabled. @Thomas, and since the problem is with a specific large file, Colomban is right it is per document, since you only need to turn off the one that is too large. @Nick, therefore not per filetype either For large files an auto-disable reparsing feature would work fine. 0002-Tell-the-user-if-real-time-reparsing-is-slow-and-let.patch: This one adds the check for the updating duration and asks the user. It is WIP, and I'm not really convinced by the dialog, as you can read in a TODO. Apart that, it seems to work pretty OK. This is more what I meant. I'm not so sure an extra dialog is needed, but if people want one that's ok. Would a don't ask me again checkbox be a good idea (if it doesn't already). @Thomas, While I still think automatic changing of settings is a last resort, and as Nick said what performance number is acceptable? If we do add it, the user must be notified, but a don't tell me again is ok (for the session) Perhaps I was wrong with 0.2s, maybe 0.5s (I didn't check Colomban's code to see what he used). @Nick no one reads the status bar, sadly A dialog is too intrusive. Consider opening 10 slow files and having to click the dialog for each one. Status bar is fine - as you sometimes say programmers don't need to be treated like newbies. Not having tag reparsing enabled will mean a few invalid bug reports at worst. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] editing big files can be too slow with tag reparsing
On 28/09/2011 12:03, Nick Treleaven wrote: @Nick, there are lots of large generated files a programmer might want to check, especially XMLs and logs, saying there is an xxxGb maximum is wrong, it depends on the machine the user has. Forcing them to have to go use another editor or to not have highlighting is bad Sorry, I missed the bit where you said no highlighting is bad. But in that case, auto-disable works fine. In any case, there is no xxx Gb maximum, I just meant that we shouldn't have a document menu option that is only for disabling reparsing when it can be done automatically. Also I'm not sure whether Scintilla handles very big files well. I don't think we need to be concerned about that. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] editing big files can be too slow with tag reparsing - HTML
On 27/09/2011 01:53, Colomban Wendling wrote: Le 27/09/2011 01:44, Lex Trotman a écrit : [...] Pentium 4, 1.9 GHz, 512 MB ram. I'm no windows expert, but isn't 512mb considered kinda marginal, 1gb better. Whats your general windows performance like? I doubt that, even on Windows, it would so drastically reduce the performances. Was your RAM completely full so it needed to swap? Windows performance is fine, I run Firefox with tons of tabs easily. I have Windows XP BTW, so 512MB is not unusually low. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] editing big files can be too slow with tag reparsing - HTML
On 26/09/2011 23:00, Colomban Wendling wrote: Le 26/09/2011 13:48, Nick Treleaven a écrit : I'm reduced to using Windows. I guess the reason is the regex code that is really old which is used for Windows builds. Maybe yeah, but I find it a little weird you see *so much* slowness... For comparison, on my machine [1] parsing and updating the list takes less than 60ms for geany.html. Just to be sure, are you seeing the 'built-in regex' debug message: 12:42:07: (null) INFO : GTK 2.22.0, GLib 2.26.0, built-in regex It might help to update it from CTags (we had a patch from Jiri Techet but it didn't look like it would build on Windows, i.e. wasn't from CTags). Maybe we could use GRegex now we have a GLib recent enough? Anyway tagamanger/regex.c looks like a few glibc source file concatenated with a 0xff separator, I'd then guess any POSIX RE implementation (e.g. today's glibc's one) would do, wouldn't it? That's probably what Jiri did, but I noticed CTags had some modifications to build on Windows. Perhaps glibc would work, but I think taking it from ctags is safer. I don't know if GRegex would work, CTags parsers may use non-POSIX features - are \w \s part of POSIX? OK... with a fresh configdir? Just tried that, same problem. Task manager showed I still had about 230MB memory free whilst having the problem - the freeze caused 100% CPU as expected. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] editing big files can be too slow with tag reparsing
On 26/09/2011 22:38, Colomban Wendling wrote: So, I'm soliciting your impressions, opinions, remarks, etc. Also, is this whole thing important enough to break string freeze less than a week before release? (read: to have some strings untranslated) Well, it's not just untranslated strings - it's best not to introduce new features close to a release in case we introduce bugs. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Hi Guys
Hi, On 27/09/2011 16:44, Jacques du Rand wrote: Quick Question: -I get a copy of source via svn compiles fine -Builds fine -Run it asgeany-svn-snapshotsrc/geany -c ~/.config/geanydev THEN : ERROR1: I can't see any open files in File Open Dialog I can see files in Project - Open dailog ? Could be related to a file filter bug - do any filters work? Are you using the latest SVN? (Try 'svn info' to find out). ERROR2:Plugins list is empty ? Probably you haven't installed Geany? ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] editing big files can be too slow with tag reparsing
On 26/09/2011 22:38, Colomban Wendling wrote: Le 24/09/2011 13:32, Lex Trotman a écrit : [...] How about automagically disabling auto-reparsing per-file if it takes too long (1s?) at runtime? Without needing to change the setting that is. I disagree, it is a bad idea to turn off a setting that changes the behavior. The user is likely to be confused by the unexpected change in behavior (new symbols no longer in autocompletes, new types not being highlighted etc). Having it automagically turn off is bad. At most, pop up a dialog suggesting turning it off and noting where, but only ever once per session. Hum, right, automagic might not be good, asking the user is always better. I've got 2 WIP patches, though they might not be appropriate to commit before release since they add strings... anyway, here they are: 0001-Per-document-real-time-symbols-setting.patch: Adds a per-document setting for real-time updates and a menu item in the Document menu. There is still a FIXME in it, feel free to give ideas :) Not sure about the use case: 1. Very big files x MB - I don't think we need to support that case specially, Geany is a programmer's editor. The user can just open as filetype none if they only want basic editing. 2. Slow tag parsers - a per-filetype setting might be more practical than per-document? I see it might be useful but I'm not convinced it's worth it. 0002-Tell-the-user-if-real-time-reparsing-is-slow-and-let.patch: This one adds the check for the updating duration and asks the user. It is WIP, and I'm not really convinced by the dialog, as you can read in a TODO. Apart that, it seems to work pretty OK. I think this is overkill - who would want editing to have ~0.2s delays? Auto-disable should be fine with a status bar message. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] editing big files can be too slow with tag reparsing - HTML
On 24/09/2011 11:59, Colomban Wendling wrote: Le 22/09/2011 18:30, Nick Treleaven a écrit : Hi, Whilst opening doc/geany.html I found it takes5s on my machine to load. It's a big document though, still perhaps the HTML tag parser performance could be improved. I think 5s is weird in the first place. I know geany.html is 6.5k-lines file, and that it may have say, 150 tags, but 5s seems way too much IMHO. Maybe we should try to find out why this is so slow on your machine, there might show a Geany bug, performance problem or something else. I'm reduced to using Windows. I guess the reason is the regex code that is really old which is used for Windows builds. It might help to update it from CTags (we had a patch from Jiri Techet but it didn't look like it would build on Windows, i.e. wasn't from CTags). * What kind of machine did you run (basically, CPU's MHz) Pentium 4, 1.9 GHz, 512 MB ram. * Had you many HTML files open, with many tags each? * Had you many non-HTML files open? Just that one file. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] TODO: update GTK+ version in Hacking
On 21/09/2011 23:06, Matthew Brush wrote: I noticed that this section[1] in the hacking manual mentions: This is because Geany depends on GTK 2.8. API symbols from newer GTK/GLib versions should be avoided to keep the source code building against GTK 2.8. Now fixed. I guess this should be updated to GTK+ 2.12 and to host that version of GTK+ API docs. [1] http://geany.org/manual/dev/hacking.html#gtk-api-documentation I'll leave this for Enrico. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
[Geany-devel] editing big files can be too slow with tag reparsing
Hi, Whilst opening doc/geany.html I found it takes 5s on my machine to load. It's a big document though, still perhaps the HTML tag parser performance could be improved. But now we have tag reparsing, editing geany.html becomes painful. I know we can disable reparsing, but what else could we do to improve the situation? It's not urgent to solve this problem, unless we want to make reparsing off by default. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] SF.net SVN: geany:[5937] trunk - use_atomic_file_saving
On 20/09/2011 23:30, Lex Trotman wrote: Rename use_safe_file_saving various pref to use_atomic_file_saving as it's not 'safe'. Use old pref if new one is missing for compatibility with Geany= 0.20. Hi Nick, I like atomic, large mushroom cloud when you use this setting:) Hehe Don't forget to update the manual Oops, now done. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
[Geany-devel] geany-commits reply subject line
Hi all, When replying to geany-commits mails, please edit the subject to include something descriptive, not just the revison number. That would really help pick out any important needles in the haystack of emails ;-) Thanks, Nick ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Various pref changes not ignored on dialog cancel
On 16/09/2011 18:11, Dimitar Zhekov wrote: Attached is the stash_tree_display(). I thought that a row changed signal will be needed, but it works without it. The renreders always reget their data when the tree is shown. No string changes. _append_pref() ignores the new action argument. Thanks, hopefully I'll commit it soon. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Various pref changes not ignored on dialog cancel
On 21/09/2011 16:31, Nick Treleaven wrote: On 16/09/2011 18:11, Dimitar Zhekov wrote: Attached is the stash_tree_display(). I thought that a row changed signal will be needed, but it works without it. The renreders always reget their data when the tree is shown. No string changes. _append_pref() ignores the new action argument. Thanks, hopefully I'll commit it soon. Committed, thanks. ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] Various pref changes not ignored on dialog cancel
--- On Thu, 15/9/11, Colomban Wendling lists@herbesfolles.org wrote: How about adding a PREF_CANCEL action to prefs.c only that will call a stash_group_cancel() to reset the values? The other I don't think that's a good design, the stash group settings should not actually be changed until ok/apply. Perhaps you meant stash_tree_cancel(), but see below. option is to clear the store, but that means recreating the various list [a visibly slow process] each time Edit - Preferences is invoked, while traversing the model is fast. What about traversing the model on reshow to reset it to the default values? Yes, that would be best. (This would be similar to how all stash prefs work with stash_group_display - the widgets are updated with the current settings). (though honestly building the view don't seem any slow to me) I didn't notice any slowness, but it would be interesting to know why if it is. Nick ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] various prefs warning use_safe_file_saving
--- On Thu, 15/9/11, Colomban Wendling lists@herbesfolles.org wrote: The dangerous settings are gio_unsafe_save_backup, use_gio_unsafe_file_saving and use_safe_file_saving, and IMHO their names are quite indicative. ... I agree, I like Dimitar's feature. I don't want to make it hard to use, just more obvious that the user should be careful. OK, so do as you think it's the best -- unless it makes the thing harder to use for really interested users I'm fine with any type of warning. Actually I've changed my mind. First - we've got a string freeze planned for this weekend. Second - the problem seems to me more that use_safe_file_saving is misleadingly named. Even with an opt-in warning, users can still be misled. The other two 'dangerous' prefs at least have 'unsafe' in the name. An opt-in warning also adds more complexity to Geany. So I propose we go ahead with the string freeze (assuming there are no other issues) and instead rename use_safe_file_saving to something less misleading. We can provide compatibility with the old name if the new name key doesn't exist. Nick ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
Re: [Geany-devel] various prefs warning - changing 'various'
--- On Fri, 16/9/11, Matthew Brush mbr...@codebrainz.ca wrote: I agree, I like Dimitar's feature. I don't want to make it hard to use, just more obvious that the user should be careful. One solution would be to just rename Various to Advanced which implies that these are not your regular basic settings. This along with a little blurb in a label inside the actual tab page should be sufficient IMO. I think that solution is too subtle - but I posted an alternative, simpler solution just now. Either way, I still don't think the wording Various Preferences should be used. It's not that it's a big deal or that the wording is incorrect per se, but Various Preferences just doesn't sound idiomatic to me (native Canadian/US English speaker). First, we have a string freeze coming up, so it should probably stay as is for now. Personally I agree that various is not the most precise term, but I think it's good enough and not worth changing. (Various is used throughout the code also). P.S. Is there any way to make the Various tab in the preferences dialog stick to the bottom position in the notebook so that when tabs are added later, Various is still at the end? Probably no enforcible way, but it would be a good convention. It could be appended last when creating the prefs dialog, and with a comment. Nick ___ Geany-devel mailing list Geany-devel@uvena.de https://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
