Quantcast

Re: Fwd: Revised Tk Text Widget

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
16 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Fwd: Revised Tk Text Widget

Steve A
Hi,
I'm glad to see Gregor release this code. It is a kind-of amazing rewrite, which i have been running extensively (and sending bug reports to Gregor) on linux for 6 months or longer, and occasionally on OS X and Win32. _Hopefully_ bugs will be minimal.

But maybe integration with the OS X build process will need some work.
Building on OS X is a little tough for me (and Gregor has no OS X experience). I have only done so using the default GNUMakefile with some additional debugging flags.... These debugging flags were part of our testing for the most of the time, and on OS X, building _without_ debugging, my app currently fails to start.

Anyway - maybe Kevin will have an easier time to build and run this than i did, as i am not much of an OS X/C dev, but i have uploaded two files that may be helpful to some.

* http://scidvspc.sourceforge.net/tmp/Wish8.6.6.cramer.tgz
Is the 8.6.6 tcl and tk (release) frameworks compiled on El Capitan with some debug info

* http://scidvspc.sourceforge.net/tmp/tk8.6.6.src.cramer.tgz
Is the tk source i use to successfully build and run with "make -f GNUMakefile" from the macos directory. It is the vanilla 8.6.6 src, with Gregor's code overlayed, plus some additional debugging tweaks added.

thanks, Steven Atkinson

On Thu, Feb 16, 2017 at 3:18 AM, Gregor Cramer <[hidden email]> wrote:
Hi Steve,

> It was the XFCC help item (i think). The other two previously problematic
> help items were the tree help window, and file finder help window, but
> after a very quick test with OS X, they seem fine too.

Okay, I remember, would be fine if it is solved.

> I posted to the list, but am not sure i have been sucessfully subscribed,

Probably you have not yet confirmed the subscription, the mailing list is
sending a confirmation email.

Cheers,
Gregor


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Tcl-Core mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/tcl-core
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Fwd: Revised Tk Text Widget

Kevin Walzer-3
On 2/15/17 2:56 PM, Steve A wrote:
>
> Anyway - maybe Kevin will have an easier time to build and run this
> than i did, as i am not much of an OS X/C dev, but i have uploaded two
> files that may be helpful to some.
I'm going to give this a try in the next few days, but what's the right
way to build this on the Mac? I've gotten a bit lost in all the back and
forth as to whether Gregor should write a TIP or not. If someone can
point me in the right direction (even if it's just linking me to the
right email in this thread) that would be great.

I'll add that I'm not aware of many TIP's in the Tk space--the last
major one I'm aware of was the themed (ttk) widgets. The rewrite of Tk
to run on Cocoa instead of Carbon was not accompanied by a TIP, for
reasons that escape me. (I think the rationale was that the public API
would not change, just the back-end implementation.) I do think a TIP
here would be useful if Gregor's existing documentation can be adapted
to the TIP format in some fashion.

Gregor, I would like to thank you for moving ahead on this, and
especially for not letting the lack of a Tk Core Team (TKT?) discourage
you. Significant improvements on Tk internals are relatively rare these
days because there are fewer developers in our community with that
expertise. After five or six years of working with Tk I barely feel
competent in the basics of the Mac's implementation. Marc Culler has
done tremendous work in moving the Mac implementation ahead where Daniel
Steffen left off some years ago.

So your contributions, in whatever form, are more than welcome.

--Kevin

--
Kevin Walzer
Code by Kevin/Mobile Code by Kevin
http://www.codebykevin.com
http://www.wtmobilesoftware.com


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Tcl-Core mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/tcl-core
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Fwd: Revised Tk Text Widget

Steve Landers
re TIP - IIRC Gregor said he wasn’t comfortable writing one. That being the case there are plenty of others (myself included) who could help.  I would hope the TIP could be relatively succinct, and highlight incompatibilities (so that we can judge the impact on existing code).   I am less concerned about detailed rationale for new features (I’m just glad that improvements are coming) but it would be good to add a brief note on each.  I most definitely would want us to avoid bikeshedding.

Re Tk internals … your experience is similar to mine.  It’s ironic that programming in Tk has mean I haven’t needed to do any significant Tk internals for over a decade, and the various platforms have moved on in that time.  

On 16 Feb 2017, 10:14 AM +0800, Kevin Walzer <[hidden email]>, wrote:
On 2/15/17 2:56 PM, Steve A wrote:

Anyway - maybe Kevin will have an easier time to build and run this
than i did, as i am not much of an OS X/C dev, but i have uploaded two
files that may be helpful to some.
I'm going to give this a try in the next few days, but what's the right
way to build this on the Mac? I've gotten a bit lost in all the back and
forth as to whether Gregor should write a TIP or not. If someone can
point me in the right direction (even if it's just linking me to the
right email in this thread) that would be great.

I'll add that I'm not aware of many TIP's in the Tk space--the last
major one I'm aware of was the themed (ttk) widgets. The rewrite of Tk
to run on Cocoa instead of Carbon was not accompanied by a TIP, for
reasons that escape me. (I think the rationale was that the public API
would not change, just the back-end implementation.) I do think a TIP
here would be useful if Gregor's existing documentation can be adapted
to the TIP format in some fashion.

Gregor, I would like to thank you for moving ahead on this, and
especially for not letting the lack of a Tk Core Team (TKT?) discourage
you. Significant improvements on Tk internals are relatively rare these
days because there are fewer developers in our community with that
expertise. After five or six years of working with Tk I barely feel
competent in the basics of the Mac's implementation. Marc Culler has
done tremendous work in moving the Mac implementation ahead where Daniel
Steffen left off some years ago.

So your contributions, in whatever form, are more than welcome.

--Kevin

--
Kevin Walzer
Code by Kevin/Mobile Code by Kevin
http://www.codebykevin.com
http://www.wtmobilesoftware.com


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Tcl-Core mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/tcl-core

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Tcl-Core mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/tcl-core
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Fwd: Revised Tk Text Widget

Steve A
In reply to this post by Kevin Walzer-3


On Thu, Feb 16, 2017 at 12:13 PM, Kevin Walzer <[hidden email]> wrote:
>
> On 2/15/17 2:56 PM, Steve A wrote:
> >
> > Anyway - maybe Kevin will have an easier time to build and run this
> > than i did, as i am not much of an OS X/C dev, but i have uploaded two
> > files that may be helpful to some.
> I'm going to give this a try in the next few days, but what's the right
> way to build this on the Mac?

It is probably easiest to just expand vanilla tcl8.6.6 src and build it with
  cd tcl8.6.6/macosx
  make -f GNUmakefile

then expand my source tarball
http://scidvspc.sourceforge.net/tmp/tk8.6.6.src.cramer.tgz
alongside tcl8.6.6 and build it the same way
 
This is the only way i know how to build wish on OS X, and gives frameworks in the ../../build directory.

My source tarball is simply the tk8.6.6 tarball with Gregor's code
expanded in the toplevel, *and* a few _hacks_ to try and enable debugging/symbols.

cheers, Steven


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Tcl-Core mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/tcl-core
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Fwd: Revised Tk Text Widget

Paul Obermeier
Am 16.02.2017 um 09:34 schrieb Steve A:


On Thu, Feb 16, 2017 at 12:13 PM, Kevin Walzer <[hidden email]> wrote:
>
> On 2/15/17 2:56 PM, Steve A wrote:
> >
> > Anyway - maybe Kevin will have an easier time to build and run this
> > than i did, as i am not much of an OS X/C dev, but i have uploaded two
> > files that may be helpful to some.
> I'm going to give this a try in the next few days, but what's the right
> way to build this on the Mac?

It is probably easiest to just expand vanilla tcl8.6.6 src and build it with
  cd tcl8.6.6/macosx
  make -f GNUmakefile

then expand my source tarball
http://scidvspc.sourceforge.net/tmp/tk8.6.6.src.cramer.tgz
alongside tcl8.6.6 and build it the same way
 
This is the only way i know how to build wish on OS X, and gives frameworks in the ../../build directory.

My source tarball is simply the tk8.6.6 tarball with Gregor's code
expanded in the toplevel, *and* a few _hacks_ to try and enable debugging/symbols.


Tried to compile your tarball on Windows, but failed.
win/Makefile.in wants a file called tkTextDispDraw.c, which is not included in the tarball (also not in Gregor's).
If I remove that dependency, I get lots of undefined reference errors regarding TkTextUndoCountUndoItems, TkTextUndoFirstRedoAtom, ...

Could you please check and supply a complete tarball.

Looking forward testing the new text widget.

Paul

cheers, Steven



------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot


_______________________________________________
Tcl-Core mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/tcl-core


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Tcl-Core mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/tcl-core
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Fwd: Revised Tk Text Widget

Steve A
Sorry Paul,

I only tested the new tk::text on win32 as an integrated package with my app
which i cross compile. (It does run great/fast :> !).

My linked tarball is only to enable _debugging_ on OS X (which, at this stage is necessary i think) and could probably be done better somehow.

Enabling a standalone win32 build of the package is still _todo_, for which a volunteer would be appreciated.

I should have made this clear... but testing this alpha software on three platforms in my spare time, makes me forget the details a little. The bulk of my testing has been on my big app on Linux - which i _do_ use daily.

Steve


On Fri, Feb 17, 2017 at 7:01 PM, Paul Obermeier <[hidden email]> wrote:
Am 16.02.2017 um 09:34 schrieb Steve A:


On Thu, Feb 16, 2017 at 12:13 PM, Kevin Walzer <[hidden email]> wrote:
>
> On 2/15/17 2:56 PM, Steve A wrote:
> >
> > Anyway - maybe Kevin will have an easier time to build and run this
> > than i did, as i am not much of an OS X/C dev, but i have uploaded two
> > files that may be helpful to some.
> I'm going to give this a try in the next few days, but what's the right
> way to build this on the Mac?

It is probably easiest to just expand vanilla tcl8.6.6 src and build it with
  cd tcl8.6.6/macosx
  make -f GNUmakefile

then expand my source tarball
http://scidvspc.sourceforge.net/tmp/tk8.6.6.src.cramer.tgz
alongside tcl8.6.6 and build it the same way
 
This is the only way i know how to build wish on OS X, and gives frameworks in the ../../build directory.

My source tarball is simply the tk8.6.6 tarball with Gregor's code
expanded in the toplevel, *and* a few _hacks_ to try and enable debugging/symbols.


Tried to compile your tarball on Windows, but failed.
win/Makefile.in wants a file called tkTextDispDraw.c, which is not included in the tarball (also not in Gregor's).
If I remove that dependency, I get lots of undefined reference errors regarding TkTextUndoCountUndoItems, TkTextUndoFirstRedoAtom, ...

Could you please check and supply a complete tarball.

Looking forward testing the new text widget.

Paul

cheers, Steven



------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot


_______________________________________________
Tcl-Core mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/tcl-core



------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Tcl-Core mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/tcl-core
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Fwd: Revised Tk Text Widget

Gregor Cramer
In reply to this post by Paul Obermeier
On Friday 17 February 2017 10:01:37 Paul Obermeier wrote:
> Tried to compile your tarball on Windows, but failed.
> win/Makefile.in wants a file called tkTextDispDraw.c, which is not
> included in the tarball (also not in Gregor's).
> If I remove that dependency, I get lots of undefined reference errors
> regarding TkTextUndoCountUndoItems, TkTextUndoFirstRedoAtom, ...
>
> Could you please check and supply a complete tarball.

I'm sorry, but I've forgotten to update the Windoze build process. I've
uploaded a new package (download
http://scidb.sourceforge.net/tk/package/tk8.6.6-revised-2017-02-17.zip). I did
a change in win/Makefile, and some changes in the sources for the support of
MSVC. But the build process for Windoze is still untested, I don't have
compilers for this platform. So please be a bit patient if the build process
needs more changes. The Windoze version is already pre-tested, but with a
cross-compiled executable (compiled under Linux).

If you like to report bugs or problems, please use the bug tracker of Scidb
(https://sourceforge.net/p/scidb/bugs/). You have to login with a Sourceforge
account before a ticket can be created.

Gregor

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Tcl-Core mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/tcl-core
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Fwd: Revised Tk Text Widget

Kevin Walzer-3
In reply to this post by Steve A
On 2/16/17 3:34 AM, Steve A wrote:


On Thu, Feb 16, 2017 at 12:13 PM, Kevin Walzer <[hidden email]> wrote:
>
> On 2/15/17 2:56 PM, Steve A wrote:
> >
> > Anyway - maybe Kevin will have an easier time to build and run this
> > than i did, as i am not much of an OS X/C dev, but i have uploaded two
> > files that may be helpful to some.
> I'm going to give this a try in the next few days, but what's the right
> way to build this on the Mac?

It is probably easiest to just expand vanilla tcl8.6.6 src and build it with
  cd tcl8.6.6/macosx
  make -f GNUmakefile

then expand my source tarball
http://scidvspc.sourceforge.net/tmp/tk8.6.6.src.cramer.tgz
alongside tcl8.6.6 and build it the same way
 

I built the archive using make -C tk/macosx and ran TkDIff against the new text implementation--it builds fine and seems to run well. As I'm not an expert with the text widget at the script level, let alone the C level, I'll let others comment on the performance differences, implementation quality, etc. But it builds well and there are no obvious regressions, which speaks well to the completeness of the implementation.

One question I wanted to pose to Gregor is if he is willing to make a commitment to be the maintainer of this code for the long term. It's difficult for someone to come into to a large code base that someone else authored and maintain it, improve it, etc. If he is willing, I suspect it would make the decision about whether to include this in a future release of Tk much easier.

--Kevin


--
Kevin Walzer
Code by Kevin/Mobile Code by Kevin
http://www.codebykevin.com
http://www.wtmobilesoftware.com

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Tcl-Core mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/tcl-core
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Fwd: Revised Tk Text Widget

Gregor Cramer
> I built the archive using make -C tk/macosx and ran TkDIff against the
> new text implementation--it builds fine and seems to run well. As I'm
> not an expert with the text widget at the script level, let alone the C
> level, I'll let others comment on the performance differences,
> implementation quality, etc. But it builds well and there are no obvious
> regressions, which speaks well to the completeness of the implementation.

I'm glad that it works, I couldn't try the Mac version by myself. In the
meanwhile we (Steve and I) did some tests under Windoze with a real
application (Scid vs PC), and it works well.

BTW: when testing with TkDiff please mind my patch (at
http://sourceforge.net/p/tkdiff/patches/28/), this application has a
synchronization problem, and with the revised version the effect of this
problem is more severe.

> One question I wanted to pose to Gregor is if he is willing to make a
> commitment to be the maintainer of this code for the long term. It's
> difficult for someone to come into to a large code base that someone
> else authored and maintain it, improve it, etc. If he is willing, I
> suspect it would make the decision about whether to include this in a
> future release of Tk much easier.

De facto François Vogel is the current maintainer, and his feedback is still
pending. IMO it would be good if the text widget has two maintainers. I think
it's clear that I'm willing to do bug fixes, and improvements if required,
otherwise the publication of the revised version wouldn't make sense. And I'm
willing to do this for a long term.

Gregor

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Tcl-Core mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/tcl-core
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Fwd: Revised Tk Text Widget

Don Hathway
  I believe I've found some memory leaks in your code.

In tkText.c
AppendScript() - Memory leak, temporary newScript not freed.
TextWidgetObjCmd() - Memory leak, temporary useIdx not freed.
TextInspectCmd() - Memory leak, temporary tagArray not freed.

I only looked for the easy ones and that may indeed not be _your_ code
but Tk code, so my apologies if I'm wrong. And apologies 2x if I'm wrong
about any those leaks :)  Your code, as has been said, is of high quality.

Also you have redefined malloc,realloc,free to ckalloc and friends. IMO
that was probably a bad idea (Ckalloc or something might've been better
and only because you cast to "void *" to save keystrokes?). That said
however, I'm not a maintainer and I don't make decisions so you can just
ignore me if you like. And I'm not suggesting you change that yourself,
I'm sure if it's an issue that someone will fix it when it's brought
into Tk.

You have brought a great gift to Tk and it speaks for itself to how much
effort you've put into it. Let me know if you'd like me to email you
privately if I find any other potential bugs in the future. Or I'll post
on a bug tracker, if you have one you're using (sorry haven't looked
atm). I'll let this one fly to the mailing list anyway in case there
should be a discussion about the malloc usage.

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Tcl-Core mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/tcl-core
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Fwd: Revised Tk Text Widget

Gregor Cramer
>   I believe I've found some memory leaks in your code.

C programming is a real crux, especially because of memory leak problems. Many
thanks for your proof-reading, I've fixed all three leaks.

> Also you have redefined malloc,realloc,free to ckalloc and friends. IMO
> that was probably a bad idea (Ckalloc or something might've been better
> and only because you cast to "void *" to save keystrokes?).

1. malloc(), realloc(), free() are the well known memory allocation functions.
These functions are not used (directly) in any of the Tk sources, so a clash
is not possible. This means that "overloading" malloc() et al shouldn't cause
problems.

2. The direct use of ckalloc(), ckrealloc(), ckfree() is producing tons of
warnings when compiling with 8.5 (I support backport to 8.5, this is still
important). The use of a superfluous cast to void* when calling one of these
functions would uglify the code (any C programmer knows about the implicit
conversion between void* and other pointer types). The bad readability of C
(compared to C++) is already a problem, programming in C was a pain for me.
But the Tk library is plain C, unfortunately.

3. This allows full support of the valuable tool valgrind (when compiled with
TK_VALGRIND=1), this is not possible when the memory handling functions of Tcl
are used.

4. The revised implementation does not belong to the Tk distribution (probably
will belong to Tk later). This solution with "defines" allows to "overload" the
allocation functions when inlining the text widget. (For example the C++
applications ScidVsPC and Scidb are inlining this widget).

Please note that solving (2) in general don't allows to redefine ckalloc() et
al, these are already "defines".

> You have brought a great gift to Tk and it speaks for itself to how much
> effort you've put into it. Let me know if you'd like me to email you
> privately if I find any other potential bugs in the future. Or I'll post
> on a bug tracker, if you have one you're using (sorry haven't looked
> atm). I'll let this one fly to the mailing list anyway in case there
> should be a discussion about the malloc usage.

Many thanks. As long as this widget is not belonging to the Tk distribution
please use the bug tracker of Scidb (https://sourceforge.net/p/scidb/bugs/).

Gregor

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Tcl-Core mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/tcl-core
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

TIP 242 on OSX

Christian Gollwitzer
In reply to this post by Kevin Walzer-3
Hi Kevin & all,

please find attached a patch which implements TIP 242 on Cocoa, which
has been missing / broken for some time.
http://www.tcl.tk/cgi-bin/tct/tip/242.html

The patch applies against core_8_6_branch.

Most of the filebox tests with user interaction (see modified
filebox.test) now work, the failures are mostly due to error messages
and different behaviour, e.g. OSX disallows selecting non-readable or
non-existent files, whereas the Unix dialog box does not.

A few other things had to be changed due to API changes in OSX.

1) From OSX 10.11, the title of the dialog box is not shown. It is
silently ignored. The -title is now prepended to the -message, which is
still shown on top. This behaviour is depending on the SDK version,
unfortunately it is inconsistent with the -mmacosx-version-min switch

2) From OSX 10.11, the accessory view is hidden behind an Options
button, and can't be opened programmatically. If the typevariable
preselects a certain filter, it's non-obvious to the user that s/he has
to open Options to make more files selectable. Therefore, I decided to
hav all allowed files selectable until the accessory view is opened

3) The OSX file panels contain an additional option "-command", which
was apparently intended for non-modal file panels, to call the callback
when the user hits open. It does not work, i.e. I've seen no effect of
-command. I propose to remove this feature, unless it will be requested
for all OSes in a TIP

4) The menu button for the filter selection is too small for usual file
types. It is obviously instantiated from pixel measures (magic numbers)
in the code. I don't know, if this can be improved, at least there
should be more room given.

5) The interaction between -filetypes, -defaultextension and
-typevariable should be thoroughly tested before merging.

Best regards,

        Christian

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Tcl-Core mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/tcl-core

typevarpatch-2017-05-12.diff (29K) Download Attachment
filebox.test.tcl (15K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: TIP 242 on OSX

Kevin Walzer-3
Christian,

On 5/12/17 3:41 AM, Christian Gollwitzer wrote:
>
> please find attached a patch which implements TIP 242 on Cocoa, which
> has been missing / broken for some time.
> http://www.tcl.tk/cgi-bin/tct/tip/242.html

Thank you for this. I will review carefully.

As this patch simply cleans up and improves the implementation of a
long-approved TIP, is any further voting by the TCT required? If not,
I'll commit after reviewing the patch and providing any necessary
feedback to Christian. Of course, any discussion during this process is
welcome.

--Kevin

--
Kevin Walzer
Code by Kevin/Mobile Code by Kevin
http://www.codebykevin.com
http://www.wtmobilesoftware.com


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Tcl-Core mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/tcl-core
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: TIP 242 on OSX

Jan Nijtmans-2
2017-05-12 12:48 GMT+02:00 Kevin Walzer:
> As this patch simply cleans up and improves the implementation of a
> long-approved TIP, is any further voting by the TCT required? If not,
> I'll commit after reviewing the patch and providing any necessary
> feedback to Christian. Of course, any discussion during this process is
> welcome.

Just go ahead! I don't see the need for voting again.

Regards,
      Jan Nijtmans

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Tcl-Core mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/tcl-core
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: TIP 242 on OSX

Kevin Walzer-3
In reply to this post by Christian Gollwitzer
Hi Christian,

I need to do some more testing, but based on my first review here is
some initial feedback on this patch:

1. First, the implementation looks very clean. You've solved a few
problems I was not sure how to approach, such as how to parse the file
types ("TCL Scripts") into something that could be displayed in the
button. Great work.

2. When running the file selector dialogs I see a lot of this in the
Terminal:

2017-05-12 08:47:38.457 Wish[89266:9714599] modal: 0

I see a couple of references to NSLog in the code. Is this debugging
stuff that can be commented out before committing? I don't want to
clutter the console with this kind of message.

3. This is a substantial patch, and I'd like to add a copyright credit
for you. How would you like that presented?

I'll probably have more questions over the next few days. Nothing yet
that needs work on your end except a response. Thanks again for this
great work!

--Kevin

--
Kevin Walzer
Code by Kevin/Mobile Code by Kevin
http://www.codebykevin.com
http://www.wtmobilesoftware.com


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Tcl-Core mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/tcl-core
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: TIP 242 on OSX

Kevin Walzer-3
In reply to this post by Christian Gollwitzer
Christian,

On 5/12/17 3:41 AM, Christian Gollwitzer wrote:
> Hi Kevin & all,
>
> please find attached a patch which implements TIP 242 on Cocoa, which
> has been missing / broken for some time.
> http://www.tcl.tk/cgi-bin/tct/tip/242.html
>
> The patch applies against core_8_6_branch.

I've committed the patch to core-8-6--branch and trunk. Thanks for your
good work here.

Kevin

--
Kevin Walzer
Code by Kevin/Mobile Code by Kevin
http://www.codebykevin.com
http://www.wtmobilesoftware.com


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Tcl-Core mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/tcl-core
Loading...