Menu
▾
▴
notion-devel
|
From: Arnout E. <no...@bz...> - 2011-01-22 15:30:39
|
Hi, As some of you might have noticed, notion git is starting to get into shape and i've been fixing some bugs here and there. >From feedback from existing Ion users, the 'elastic/proportional' tab size functionality that was merged into Notion from http://github.com/gwash/ion-3plus is not appreciated by everyone. I don't care for it much myself either, and the code is not very neat (actually one of the things I fixed was an endless loop in this code, causing notion to crash and fill up a CPU). We should at least make this behavior optional - would anyone be against just reverting it entirely? Kind regards, Arnout |
|
From: ebik <eb...@dr...> - 2011-01-22 17:39:53
Attachments:
signature.asc
|
Hmm, I think I'm to blame. The patch was proof of concept, i.e. ugly code, written in a lack of time. I have never got to "clean" it. It was not mentioned to /integrate/ into stable version, but to review and eventualy rewrite. To be precise, I submited it in this email, where is also documented the behavior of the tabs: http://www.mail-archive.com/ion...@li.../msg03125.html (Sending for second time. First time I used email which is not in list). On Sat, 22 Jan 2011 16:30:24 +0100 Arnout Engelen <no...@bz...> wrote: > Hi, > > As some of you might have noticed, notion git is starting to get into > shape and i've been fixing some bugs here and there. > > >From feedback from existing Ion users, the 'elastic/proportional' > >tab size > functionality that was merged into Notion from > http://github.com/gwash/ion-3plus is not appreciated by everyone. I > don't care for it much myself either, and the code is not very neat > (actually one of the things I fixed was an endless loop in this code, > causing notion to crash and fill up a CPU). > > We should at least make this behavior optional - would anyone be > against just reverting it entirely? > > > Kind regards, > > Arnout > > ------------------------------------------------------------------------------ > Special Offer-- Download ArcSight Logger for FREE (a $49 USD value)! > Finally, a world-class log management solution at an even better > price-free! Download using promo code Free_Logger_4_Dev2Dev. Offer > expires February 28th, so secure your free ArcSight Logger TODAY! > http://p.sf.net/sfu/arcsight-sfd2d > _______________________________________________ > Notion-devel mailing list > Not...@li... > https://lists.sourceforge.net/lists/listinfo/notion-devel > -- Tomáš 'ebík' Ebenlendr PF 2011.05963292745 |
|
From: <eb...@dr...> - 2011-01-27 18:33:56
Attachments:
signature.asc
|
I inspected your commit, it was hard to find the fix. Your commit shoud be split in two commits: first one - code purifying (indenting, variable renames etc.) second one - the actual fix This way it would be clear what you did. I like ion3(plus) because of every line of code was critically considered by Tuomo. Now we have no Tuomo, thus we should review the patches. And not blindly apply them. This includes all user contributed patches that were not part of ion3plus (including my patch for proportional tabs). Otherwise we end with unmaintainable code. On Sat, 22 Jan 2011 16:30:24 +0100 Arnout Engelen <no...@bz...> wrote: > Hi, > > As some of you might have noticed, notion git is starting to get into > shape and i've been fixing some bugs here and there. > > >From feedback from existing Ion users, the 'elastic/proportional' > >tab size > functionality that was merged into Notion from > http://github.com/gwash/ion-3plus is not appreciated by everyone. I > don't care for it much myself either, and the code is not very neat > (actually one of the things I fixed was an endless loop in this code, > causing notion to crash and fill up a CPU). > > We should at least make this behavior optional - would anyone be > against just reverting it entirely? > > > Kind regards, > > Arnout > > ------------------------------------------------------------------------------ > Special Offer-- Download ArcSight Logger for FREE (a $49 USD value)! > Finally, a world-class log management solution at an even better > price-free! Download using promo code Free_Logger_4_Dev2Dev. Offer > expires February 28th, so secure your free ArcSight Logger TODAY! > http://p.sf.net/sfu/arcsight-sfd2d > _______________________________________________ > Notion-devel mailing list > Not...@li... > https://lists.sourceforge.net/lists/listinfo/notion-devel > -- Tomáš 'ebík' Ebenlendr PF 2011.07332182268 |
|
From: <eb...@dr...> - 2011-01-27 20:44:22
Attachments:
signature.asc
|
Okay reverting patches can be found here: http://drak.ucw.cz/~ebik/git/notion see commit log below. If I will have time I will create new version of the proportional tabs code. At least more configurable and with nicer algorithm. I also re-commited one fix and one default found in the reverted patch. The remaining difference is discutable. Either it removes unnecessary code or it is a regression. The difference is below. ============ not applied fix =========== diff --git a/etc/look.lua b/etc/look.lua new file mode 100644 index 0000000..2484e42 --- /dev/null +++ b/etc/look.lua @@ -0,0 +1 @@ +dopath('look_newviolet') diff --git a/ioncore/sizepolicy.c b/ioncore/sizepolicy.c index ce77edb..8c42327 100644 --- a/ioncore/sizepolicy.c +++ b/ioncore/sizepolicy.c @@ -360,10 +360,5 @@ bool string2sizepolicy(const char *szplcy, WSizePolicy *value) const char *sizepolicy2string(WSizePolicy szplcy) { - const char* str=stringintmap_key(szplcy_specs, szplcy, NULL); - if(str==NULL){ - /* fall back on policy without modifiers if full name not found */ - str=stringintmap_key(szplcy_specs, szplcy&0xff, NULL); - } - return str; + return stringintmap_key(szplcy_specs, szplcy, NULL); } ======================================= Here is commit log of changes I made. The 'Add default style' change is also based on the reverted patches. ============= commit log ============== commit 7f09147bb8689b745dfec7aaac33eb1655d44a0d Author: Tomáš Ebenlendr <eb...@uc...> Date: Thu Jan 27 21:33:03 2011 +0100 Add default style. commit 4e896d7897131c1c10c8211d8dcf3cff3aa567d7 Author: Tomáš Ebenlendr <eb...@uc...> Date: Thu Jan 27 21:03:08 2011 +0100 Fix str_len() in the case of multibyte encoding. str_len() fix based on reverted commit 951f7ac - changes found in http://github.com/gwash/ion-3plus commit d8196fff51d4d2a14646239e211ff1cd788c9883 Author: Tomáš Ebenlendr <eb...@uc...> Date: Thu Jan 27 20:00:12 2011 +0100 Revert of 951f7acb - because of ugly code. Commit 951f7acb with name: "Merge some changes found in http://github.com/gwash/ion-3plus" contains my (eb...@uc...) patch to elastic/proportional tab sizes, which was proof of concept and was not intended to get into mainline. commit 7e3df042741bb89eb26b2a0bcf4aa7a68436045e Author: Tomáš Ebenlendr <eb...@uc...> Date: Thu Jan 27 19:52:59 2011 +0100 Reverting commit 8cabc655, to revert 951f7acb Reverting commit 8cabc655 (fix of miscalculations of tab sizes) to be able to revert commit 951f7acb, which was a blind apply of changes from http://github.com/gwash/ion3-plus without any review. Second reason is that 8cabc655 should be split in two commits. -- Tomáš 'ebík' Ebenlendr PF 2011.07367757484 |
|
From: Arnout E. <no...@bz...> - 2011-01-27 21:04:28
|
On Thu, Jan 27, 2011 at 07:33:47PM +0100, eb...@dr... wrote: > I inspected your commit, That'd be the one at http://notion.git.sourceforge.net/git/gitweb.cgi?p=notion/notion;a=commit;h=8cabc6558530b6eaf126f0d57c92a719a7284b83 > it was hard to find the fix. > > Your commit shoud be split in two commits: > first one - code purifying (indenting, variable renames etc.) > second one - the actual fix > This way it would be clear what you did. Agreed entirely. > I like ion3(plus) because of every line of code was critically > considered by Tuomo. Now we have no Tuomo, thus we should review the > patches. And not blindly apply them. Agreed. All committers should review any patches before applying them, and discuss on the mailinglist in case of doubts. It's useful to also check the work done by committers, and I absolutely welcome you looking into this. For now we don't have many reviewers (afaics you're one of the relatively few people here with the skills required, and you mentioned earlier you don't have much time for this). Because of this I think it would be unpractical to require that work done by a committer himself is always first reviewed by a second reviewer before being committed. It's up to the committer to take responsibility and first consult the mailinglists before making changes that might be controversial - hence the post that started this thread ;). Aside from that commits can of course be reviewed after commiting. > This includes all user contributed patches that were not part of ion3plus > (including my patch for proportional tabs). Yes - I wrongfully assumed the git repo set up by gwash contained only code already vetted by Tuomo, no additional patches. I should have been more picky there, sorry about that. Kind regards, Arnout > On Sat, 22 Jan 2011 16:30:24 +0100 > Arnout Engelen <no...@bz...> wrote: > > Hi, > > > > As some of you might have noticed, notion git is starting to get into > > shape and i've been fixing some bugs here and there. > > > > >From feedback from existing Ion users, the 'elastic/proportional' > > >tab size > > functionality that was merged into Notion from > > http://github.com/gwash/ion-3plus is not appreciated by everyone. I > > don't care for it much myself either, and the code is not very neat > > (actually one of the things I fixed was an endless loop in this code, > > causing notion to crash and fill up a CPU). > > > > We should at least make this behavior optional - would anyone be > > against just reverting it entirely? > > > > > > Kind regards, > > > > Arnout > > > > ------------------------------------------------------------------------------ > > Special Offer-- Download ArcSight Logger for FREE (a $49 USD value)! > > Finally, a world-class log management solution at an even better > > price-free! Download using promo code Free_Logger_4_Dev2Dev. Offer > > expires February 28th, so secure your free ArcSight Logger TODAY! > > http://p.sf.net/sfu/arcsight-sfd2d > > _______________________________________________ > > Notion-devel mailing list > > Not...@li... > > https://lists.sourceforge.net/lists/listinfo/notion-devel > > > > > -- > Tomáš 'ebík' Ebenlendr > PF 2011.07332182268 > > ------------------------------------------------------------------------------ > Special Offer-- Download ArcSight Logger for FREE (a $49 USD value)! > Finally, a world-class log management solution at an even better price-free! > Download using promo code Free_Logger_4_Dev2Dev. Offer expires > February 28th, so secure your free ArcSight Logger TODAY! > http://p.sf.net/sfu/arcsight-sfd2d > _______________________________________________ > Notion-devel mailing list > Not...@li... > https://lists.sourceforge.net/lists/listinfo/notion-devel |
|
From: Arnout E. <no...@bz...> - 2011-01-27 21:26:09
|
On Thu, Jan 27, 2011 at 09:44:13PM +0100, eb...@dr... wrote: > Okay reverting patches can be found here: > http://drak.ucw.cz/~ebik/git/notion > see commit log below. Thanks! I looked over the commits and tested them. All seems OK, so I pushed them to sf.net git. > If I will have time I will create new version of the proportional tabs code. > At least more configurable and with nicer algorithm. OK > I also re-commited one fix and one default found in the reverted patch. Yes, looks good. > The remaining difference is discutable. Either it removes unnecessary code > or it is a regression. The difference is below. Yes - it doesn't seem harmful to have a fall-back scenario in case stringintmap_key returns NULL, so unless we get any new information it seems to make sense to keep that code. Kind regards, Arnout > ============ not applied fix =========== > diff --git a/etc/look.lua b/etc/look.lua > new file mode 100644 > index 0000000..2484e42 > --- /dev/null > +++ b/etc/look.lua > @@ -0,0 +1 @@ > +dopath('look_newviolet') > diff --git a/ioncore/sizepolicy.c b/ioncore/sizepolicy.c > index ce77edb..8c42327 100644 > --- a/ioncore/sizepolicy.c > +++ b/ioncore/sizepolicy.c > @@ -360,10 +360,5 @@ bool string2sizepolicy(const char *szplcy, WSizePolicy *value) > > const char *sizepolicy2string(WSizePolicy szplcy) > { > - const char* str=stringintmap_key(szplcy_specs, szplcy, NULL); > - if(str==NULL){ > - /* fall back on policy without modifiers if full name not found */ > - str=stringintmap_key(szplcy_specs, szplcy&0xff, NULL); > - } > - return str; > + return stringintmap_key(szplcy_specs, szplcy, NULL); > } > > ======================================= > > > Here is commit log of changes I made. > The 'Add default style' change is also based on the reverted patches. > > ============= commit log ============== > commit 7f09147bb8689b745dfec7aaac33eb1655d44a0d > Author: Tomáš Ebenlendr <eb...@uc...> > Date: Thu Jan 27 21:33:03 2011 +0100 > > Add default style. > > commit 4e896d7897131c1c10c8211d8dcf3cff3aa567d7 > Author: Tomáš Ebenlendr <eb...@uc...> > Date: Thu Jan 27 21:03:08 2011 +0100 > > Fix str_len() in the case of multibyte encoding. > > str_len() fix based on reverted commit 951f7ac - changes found in > http://github.com/gwash/ion-3plus > > commit d8196fff51d4d2a14646239e211ff1cd788c9883 > Author: Tomáš Ebenlendr <eb...@uc...> > Date: Thu Jan 27 20:00:12 2011 +0100 > > Revert of 951f7acb - because of ugly code. > > Commit 951f7acb with name: > "Merge some changes found in http://github.com/gwash/ion-3plus" > contains my (eb...@uc...) patch to elastic/proportional tab sizes, > which was proof of concept and was not intended to get into > mainline. > > commit 7e3df042741bb89eb26b2a0bcf4aa7a68436045e > Author: Tomáš Ebenlendr <eb...@uc...> > Date: Thu Jan 27 19:52:59 2011 +0100 > > Reverting commit 8cabc655, to revert 951f7acb > > Reverting commit 8cabc655 (fix of miscalculations of tab sizes) to > be able to revert commit 951f7acb, which was a blind apply of > changes from http://github.com/gwash/ion3-plus without any review. > > Second reason is that 8cabc655 should be split in two commits. > > > -- > Tomáš 'ebík' Ebenlendr > PF 2011.07367757484 > > ------------------------------------------------------------------------------ > Special Offer-- Download ArcSight Logger for FREE (a $49 USD value)! > Finally, a world-class log management solution at an even better price-free! > Download using promo code Free_Logger_4_Dev2Dev. Offer expires > February 28th, so secure your free ArcSight Logger TODAY! > http://p.sf.net/sfu/arcsight-sfd2d > _______________________________________________ > Notion-devel mailing list > Not...@li... > https://lists.sourceforge.net/lists/listinfo/notion-devel |
|
From: Ole J. B. <ole...@ya...> - 2011-01-28 13:25:18
|
>> The remaining difference is discutable. Either it removes unnecessary code >> or it is a regression. The difference is below. >Yes - it doesn't seem harmful to have a fall-back scenario in case > stringintmap_key returns NULL, so unless we get any new information it seems > to make sense to keep that code. > That fixes the issue of the sp sometimes being impossible to resize after a reboot. It's not a perfect fix, but was easy to do compared to providing a string value for every size policy combination. http://lists.berlios.de/pipermail/ion-general/2009-December/001775.html - Ole Jørgen Brønner |
|
From: kevin g. <kev...@gm...> - 2011-01-27 22:37:39
|
On Thu, Jan 27, 2011 at 3:04 PM, Arnout Engelen <no...@bz...> wrote: > On Thu, Jan 27, 2011 at 07:33:47PM +0100, eb...@dr... wrote: >> I inspected your commit, > > That'd be the one at http://notion.git.sourceforge.net/git/gitweb.cgi?p=notion/notion;a=commit;h=8cabc6558530b6eaf126f0d57c92a719a7284b83 > >> it was hard to find the fix. >> >> Your commit shoud be split in two commits: >> first one - code purifying (indenting, variable renames etc.) >> second one - the actual fix >> This way it would be clear what you did. > > Agreed entirely. > >> I like ion3(plus) because of every line of code was critically >> considered by Tuomo. Now we have no Tuomo, thus we should review the >> patches. And not blindly apply them. > > Agreed. All committers should review any patches before applying them, and > discuss on the mailinglist in case of doubts. > > It's useful to also check the work done by committers, and I absolutely welcome > you looking into this. > > For now we don't have many reviewers (afaics you're one of the relatively few > people here with the skills required, and you mentioned earlier you don't have > much time for this). Because of this I think it would be unpractical to > require that work done by a committer himself is always first reviewed by a > second reviewer before being committed. It's up to the committer to take > responsibility and first consult the mailinglists before making changes that > might be controversial - hence the post that started this thread ;). Aside > from that commits can of course be reviewed after commiting. Perhaps patches should go to the mailing list regardless so that potential reviewers (I'm actually thinking of myself here) can look them over as time is available. Ideally they would get acked before being committed, but there is value in late reviews as well. Also it provides a resource where potential developers can become familiar with project norms and issues. Kevin Granade > >> This includes all user contributed patches that were not part of ion3plus >> (including my patch for proportional tabs). > > Yes - I wrongfully assumed the git repo set up by gwash contained only code > already vetted by Tuomo, no additional patches. I should have been more picky > there, sorry about that. > > > Kind regards, > > Arnout > >> On Sat, 22 Jan 2011 16:30:24 +0100 >> Arnout Engelen <no...@bz...> wrote: >> > Hi, >> > >> > As some of you might have noticed, notion git is starting to get into >> > shape and i've been fixing some bugs here and there. >> > >> > >From feedback from existing Ion users, the 'elastic/proportional' >> > >tab size >> > functionality that was merged into Notion from >> > http://github.com/gwash/ion-3plus is not appreciated by everyone. I >> > don't care for it much myself either, and the code is not very neat >> > (actually one of the things I fixed was an endless loop in this code, >> > causing notion to crash and fill up a CPU). >> > >> > We should at least make this behavior optional - would anyone be >> > against just reverting it entirely? >> > >> > >> > Kind regards, >> > >> > Arnout >> > >> > ------------------------------------------------------------------------------ >> > Special Offer-- Download ArcSight Logger for FREE (a $49 USD value)! >> > Finally, a world-class log management solution at an even better >> > price-free! Download using promo code Free_Logger_4_Dev2Dev. Offer >> > expires February 28th, so secure your free ArcSight Logger TODAY! >> > http://p.sf.net/sfu/arcsight-sfd2d >> > _______________________________________________ >> > Notion-devel mailing list >> > Not...@li... >> > https://lists.sourceforge.net/lists/listinfo/notion-devel >> > >> >> >> -- >> Tomáš 'ebík' Ebenlendr >> PF 2011.07332182268 >> > > > >> ------------------------------------------------------------------------------ >> Special Offer-- Download ArcSight Logger for FREE (a $49 USD value)! >> Finally, a world-class log management solution at an even better price-free! >> Download using promo code Free_Logger_4_Dev2Dev. Offer expires >> February 28th, so secure your free ArcSight Logger TODAY! >> http://p.sf.net/sfu/arcsight-sfd2d >> _______________________________________________ >> Notion-devel mailing list >> Not...@li... >> https://lists.sourceforge.net/lists/listinfo/notion-devel > > > ------------------------------------------------------------------------------ > Special Offer-- Download ArcSight Logger for FREE (a $49 USD value)! > Finally, a world-class log management solution at an even better price-free! > Download using promo code Free_Logger_4_Dev2Dev. Offer expires > February 28th, so secure your free ArcSight Logger TODAY! > http://p.sf.net/sfu/arcsight-sfd2d > _______________________________________________ > Notion-devel mailing list > Not...@li... > https://lists.sourceforge.net/lists/listinfo/notion-devel > |
|
From: Arnout E. <no...@bz...> - 2011-01-28 11:55:30
|
On Thu, Jan 27, 2011 at 04:37:08PM -0600, kevin granade wrote: > On Thu, Jan 27, 2011 at 3:04 PM, Arnout Engelen <no...@bz...> wrote: > > It's useful to also check the work done by committers, and I absolutely welcome > > you looking into this. > > > > For now we don't have many reviewers (afaics you're one of the relatively few > > people here with the skills required, and you mentioned earlier you don't have > > much time for this). Because of this I think it would be unpractical to > > require that work done by a committer himself is always first reviewed by a > > second reviewer before being committed. It's up to the committer to take > > responsibility and first consult the mailinglists before making changes that > > might be controversial - hence the post that started this thread ;). Aside > > from that commits can of course be reviewed after commiting. > > Perhaps patches should go to the mailing list regardless so that potential > reviewers can look them over as time is available. A (seperate) 'notion-commits' mailinglists would be nice. Until that's set up, you can track http://notion.git.sourceforge.net/git/gitweb.cgi?p=notion/notion;a=rss or https://sourceforge.net/export/rss2_keepsake.php?group_id=314802 You can also join the #notion IRC channel on irc.freenode.net, commits are announced and often discussed there. > (I'm actually thinking of myself here) Feel free! Kind regards, Arnout |
|
From: kevin g. <kev...@gm...> - 2011-01-28 15:49:43
|
On Fri, Jan 28, 2011 at 5:55 AM, Arnout Engelen <no...@bz...> wrote: > On Thu, Jan 27, 2011 at 04:37:08PM -0600, kevin granade wrote: >> On Thu, Jan 27, 2011 at 3:04 PM, Arnout Engelen <no...@bz...> wrote: >> > It's useful to also check the work done by committers, and I absolutely welcome >> > you looking into this. >> > >> > For now we don't have many reviewers (afaics you're one of the relatively few >> > people here with the skills required, and you mentioned earlier you don't have >> > much time for this). Because of this I think it would be unpractical to >> > require that work done by a committer himself is always first reviewed by a >> > second reviewer before being committed. It's up to the committer to take >> > responsibility and first consult the mailinglists before making changes that >> > might be controversial - hence the post that started this thread ;). Aside >> > from that commits can of course be reviewed after commiting. >> >> Perhaps patches should go to the mailing list regardless so that potential >> reviewers can look them over as time is available. > > A (seperate) 'notion-commits' mailinglists would be nice. > > Until that's set up, you can track > http://notion.git.sourceforge.net/git/gitweb.cgi?p=notion/notion;a=rss or > https://sourceforge.net/export/rss2_keepsake.php?group_id=314802 > Ah! Thanks for that pointer. I'll take advantage of it. > You can also join the #notion IRC channel on irc.freenode.net, commits are > announced and often discussed there. > >> (I'm actually thinking of myself here) > > Feel free! > > > Kind regards, > > Arnout > |
|
From: Arnout E. <no...@bz...> - 2011-02-19 01:22:20
|
On Fri, Jan 28, 2011 at 12:55:18PM +0100, Arnout Engelen wrote: > On Thu, Jan 27, 2011 at 04:37:08PM -0600, kevin granade wrote: > > Perhaps patches should go to the mailing list regardless so that potential > > reviewers can look them over as time is available. > > A (seperate) 'notion-commits' mailinglists would be nice. It's now all set up: https://lists.sourceforge.net/lists/listinfo/notion-commits Kind regards, Arnout |
×
Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.
Thanks for helping keep SourceForge clean.
X