WIP: Extend meaning of "Hidden Files" (PR version 4) #209

Draft
VinceR wants to merge 4 commits from issue/270/tdelibs-V4 into master
Collaborator

This is a continuation of PR #179 and is intended to eventually address issue TDE/tdebase#270. To do that, changes to the following additional files will need to be pushed as well as corresponding changes to tdebase/libkonq and tdebase/konqueror:

  tdeio/tdeio/kdirlister.h
  tdeio/tdeio/kdirlister.cpp
  tdeio/tdeio/kdirlister_p.h
  tdeio/tdeio/tdefilefilter.h
  tdeio/tdeio/tdefilefilter.cpp
  tdeio/tdefile/tdediroperator.cpp
  tdeio/tdefile/tdefiletreebranch.cpp

All code has been tested successfully.

This is a continuation of PR #179 and is intended to eventually address issue TDE/tdebase#270. To do that, changes to the following additional files will need to be pushed as well as corresponding changes to `tdebase/libkonq` and `tdebase/konqueror`: ``` tdeio/tdeio/kdirlister.h tdeio/tdeio/kdirlister.cpp tdeio/tdeio/kdirlister_p.h tdeio/tdeio/tdefilefilter.h tdeio/tdeio/tdefilefilter.cpp tdeio/tdefile/tdediroperator.cpp tdeio/tdefile/tdefiletreebranch.cpp ``` All code has been tested successfully.
VinceR added this to the R14.2.x milestone 11 months ago
VinceR added the PR/wip label 11 months ago
MicheleC was assigned by VinceR 11 months ago
VinceR reviewed 11 months ago
//typedef wchar_t CHAR16;
//typedef unsigned short CHAR16;
typedef TQChar CHAR16;
Poster
Collaborator

would the code still work if we use TQChar instead of w_char_t? Nothing against the latter, but given that TDE is heavily coded using TQt3 classes, it would be more natural to use TQChar if the functionality is not affected.

I did follow your suggestion to use TQChar instead of an integer representing a code point but I still wonder if might be less efficient. Anyway, all three typedefs work.

>would the code still work if we use TQChar instead of w_char_t? Nothing against the latter, but given that TDE is heavily coded using TQt3 classes, it would be more natural to use TQChar if the functionality is not affected. I did follow your suggestion to use `TQChar` instead of an integer representing a code point but I still wonder if might be less efficient. Anyway, all three typedefs work.
Owner

TQChar uses a ushort as internal representation, so there should be no differences in terms of efficiently, but the readibility improves because TQStrings are made up of TQChars, not ushorts.

`TQChar` uses a `ushort` as [internal representation](https://mirror.git.trinitydesktop.org/gitea/TDE/tqt3/src/branch/master/src/tools/ntqstring.h#L248), so there should be no differences in terms of efficiently, but the readibility improves because TQStrings are made up of TQChars, not ushorts.
MicheleC marked this conversation as resolved
VinceR reviewed 11 months ago
const defaultCollation EquivalentsTable // terminating ';' is provided in include file
#include "tequivchars-mapping.h"
uint EquivTableROWS = sizeof(EquivalentsTable)/sizeof(EquivalentsTable[0]);
Poster
Collaborator

As you can see, the table is now an array of structs versus a 2 dimensional array.

I defined EquivalentsTable the way I did so that I could keep the name declaration in this file but the table content and hard-coded table dimensions in the included file.

What I really would have like to do was something like this:

const defaultCollation EquivalentsTable[] = {
  #include "tequivchars-mapping.h"
};

and let the compiler figure out the array size. I could not get anything like that to compile.

As you can see, the table is now an array of structs versus a 2 dimensional array. I defined `EquivalentsTable` the way I did so that I could keep the name declaration in this file but the table content and hard-coded table dimensions in the included file. What I really would have like to do was something like this: ``` const defaultCollation EquivalentsTable[] = { #include "tequivchars-mapping.h" }; ``` and let the compiler figure out the array size. I could not get anything like that to compile.
VinceR reviewed 11 months ago
continue;
}
#endif
Poster
Collaborator

I thought this might be a worthwhile optimization, requiring only 3 comparisons to process upper case ASCII letters and only 1 comparison for all other ASCII characters.

After digging into TQt source code, I noticed that it internally utilizes full (non-sparse) tables of various unicode properties (src/tools/qunicodetables.cpp). So another optimization I could try is something like this:

  if ( ! currChar.isLetterOrNumber() )
    continue;
I thought this might be a worthwhile optimization, requiring only 3 comparisons to process upper case ASCII letters and only 1 comparison for all other ASCII characters. After digging into TQt source code, I noticed that it internally utilizes full (non-sparse) tables of various unicode properties (`src/tools/qunicodetables.cpp`). So another optimization I could try is something like this: ``` if ( ! currChar.isLetterOrNumber() ) continue; ```
Owner

ok

ok
MicheleC marked this conversation as resolved
VinceR reviewed 11 months ago
else
high = mid - 1;
}
//-Debug: std::cerr << TQString(currChar).utf8().data() << "'" << std::endl;
Poster
Collaborator

bsearch() from stdlib does that, so you may want to take a look.

I looked at that but didn't like the need for a callback function for each comparison. Also, the callback function expects a return code of <1, 0, or >1. Unlike integers, there is no simple TQChar operation that can quickly compute that. I suspect that using bsearch would end up taking more lines of code.

>bsearch() from stdlib does that, so you may want to take a look. I looked at that but didn't like the need for a callback function for each comparison. Also, the callback function expects a return code of <1, 0, or >1. Unlike integers, there is no simple `TQChar` operation that can quickly compute that. I suspect that using `bsearch` would end up taking more lines of code.
Owner

The callback function is a standard way to allow using the same algorithm on many different types of data. It provides design flexibility at the expense of a marginal runtime overhead. Implementation of the function is logically no different from the comparison done in the code above, where TQChars get compared for their partial ordering.
Anyway current code is fine and there is no need to move to bsearch.

The callback function is a standard way to allow using the same algorithm on many different types of data. It provides design flexibility at the expense of a marginal runtime overhead. Implementation of the function is logically no different from the comparison done in the code above, where TQChars get compared for their partial ordering. Anyway current code is fine and there is no need to move to `bsearch`.
MicheleC marked this conversation as resolved
Owner

Will review the code this week, possibly tomorrow or Wednesday

Will review the code this week, possibly tomorrow or Wednesday
MicheleC changed title from WIP: Address issue 270 - Extend meaning of "Hidden Files" (PR version 4) to WIP: Address issue TDE/tdebase#270 - Extend meaning of "Hidden Files" (PR version 4) 11 months ago
MicheleC changed title from WIP: Address issue TDE/tdebase#270 - Extend meaning of "Hidden Files" (PR version 4) to WIP: Extend meaning of "Hidden Files" (PR version 4) 11 months ago
MicheleC requested changes 10 months ago
MicheleC left a comment
Owner

Next round of feedback :-) Sorry for the delay, finally come around to review the code.

Next round of feedback :-) Sorry for the delay, finally come around to review the code.
SOURCES ${${target}_SRCS}
VERSION 14.1.0
EMBED tdecorenetwork-static
LINK DCOP-shared tdefx-shared ICE SM ${ZLIB_LIBRARIES} ${RESOLV_LIBRARIES}
Owner

${RESOLV_LIBRARIES} was introduce in a recent commit (2 months ago)m, probably you missed that in your local copy. Always make sure to compared against latest master :-)

`${RESOLV_LIBRARIES}` was introduce in a recent commit (2 months ago)m, probably you missed that in your local copy. Always make sure to compared against latest master :-)
Poster
Collaborator

${RESOLV_LIBRARIES} was introduce in a recent commit (2 months ago)m, probably you missed that in your local copy. Always make sure to compared against latest master :-)

Sorry, I missed that and I am unable to figure out why that happened.

>`${RESOLV_LIBRARIES}` was introduce in a recent commit (2 months ago)m, probably you missed that in your local copy. Always make sure to compared against latest master :-) Sorry, I missed that and I am unable to figure out why that happened.
#include <tdeconfig.h>
#include <tdelocale.h>
#include <kcharsets.h>
#include <tequivchars.h>
Owner

Include of tequivchars.h is no longer required see comments below)

Include of tequivchars.h is no longer required see comments below)
return _charsets;
}
TEquivChars *TDEGlobal::equivChars()
Owner

No longer required, see comments in tdecore/tequivchars.h about making TEquivChars::replaceChars a static method

No longer required, see comments in `tdecore/tequivchars.h` about making `TEquivChars::replaceChars` a static method
TDEInstance *TDEGlobal::_activeInstance = 0;
TDELocale *TDEGlobal::_locale = 0;
KCharsets *TDEGlobal::_charsets = 0;
TEquivChars *TDEGlobal::_equivChars = 0;
Owner

TDEGlobal::_equivChars is also not required.

`TDEGlobal::_equivChars` is also not required.
#endif
class TDELocale;
class TDEStringMatcher;
class TEquivChars;
Owner

class TEquivChars; not required here.

`class TEquivChars;` not required here.
* The global alphanumeric character equivalence mapper.
* @return the global alphanumeric character equivalence mapper.
*/
static TEquivChars *equivChars();
Owner

No longer required, see comments in tdecore/tequivchars.h about making TEquivChars::replaceChars a static method

No longer required, see comments in `tdecore/tequivchars.h` about making `TEquivChars::replaceChars` a static method
static TDELocale *_locale;
static KCharsets *_charsets;
static TDEStringMatcher *_hiddenFileMatcher;
static TEquivChars *_equivChars;
Owner

_equivChars not required here.

`_equivChars` not required here.
#include <features.h>
#ifdef __GLIBC__
#include <fnmatch.h>
Owner

This will need detection from cmake file and conditional code inclusion. Ok to do it later, but before we merge the code.

This will need detection from cmake file and conditional code inclusion. Ok to do it later, but before we merge the code.
Poster
Collaborator

This will need detection from cmake file and conditional code inclusion. Ok to do it later, but before we merge the code.

I see the problem. <features.h> is not guaranteed to always exist. I replaced the above code with:

#if __has_include( <features.h> ) // C++17
#pragma message "Using features.h to check for __GLIBC__"
#include <features.h>
#endif

#ifdef __GLIBC__
#include <fnmatch.h>
#pragma message "TSM using GLIBC fnmatch() for wildcard matching"
#endif

Do you think that is sufficient?

>This will need detection from cmake file and conditional code inclusion. Ok to do it later, but before we merge the code. I see the problem. `<features.h>` is not guaranteed to always exist. I replaced the above code with: ``` #if __has_include( <features.h> ) // C++17 #pragma message "Using features.h to check for __GLIBC__" #include <features.h> #endif #ifdef __GLIBC__ #include <fnmatch.h> #pragma message "TSM using GLIBC fnmatch() for wildcard matching" #endif ``` Do you think that is sufficient?
namespace TSM {
class AuxData
Owner

One of the member is a pointer, so better to provide a destructor to clear allocated memory. Also better to provide a copy constructor since we are copying an AuxData later in the code.

Have you verified there are no memory leaks with the matchEngine in all possible scenarios (exceptions included)? I would (and will before merging) have to verify that as well, but good if you think about it first.

One of the member is a pointer, so better to provide a destructor to clear allocated memory. Also better to provide a copy constructor since we are copying an `AuxData` later in the code. Have you verified there are no memory leaks with the `matchEngine` in all possible scenarios (exceptions included)? I would (and will before merging) have to verify that as well, but good if you think about it first.
Poster
Collaborator

One of the member is a pointer, so better to provide a destructor to clear allocated memory.

An AuxData object functions more like a struct than a class although we use a constructor to start with sane values. The object is meaningful only as a member of an AuxDataList, which in turn is a member of a TDEStringMatcherPrivate class. The function TDEStringMatcherPrivate::clearAll() has sole responsibility for freeing any dynamically allocated memory associated with each member of its AuxDataList before clearing the list itself.

Also better to provide a copy constructor since we are copying an AuxData later in the code.

I assume that you are referring to copying a TDEStringMatcherPrivate object? I’m not sure what I would do in this method that isn’t already done by standard C++ copying.

Have you verified there are no memory leaks with the matchEngine in all possible scenarios (exceptions included)?

Verified … all possible scenarios … I can’t say that. But I have made sure that clearAll() is always called before before any return from setMatchSpecs(). And inspecting the output of TSMTRACE reveals that there is always a corresponding ‘free’ that follows a regex engine ‘allocate’ for all cases that I have tested.

I would (and will before merging) have to verify that as well, but good if you think about it first.

I just installed valgrind (which I hadn't realized was FOSS). Is that the tool of choice?

>One of the member is a pointer, so better to provide a destructor to clear allocated memory. An `AuxData` object functions more like a `struct` than a `class` although we use a constructor to start with sane values. The object is meaningful only as a member of an `AuxDataList`, which in turn is a member of a `TDEStringMatcherPrivate` class. The function `TDEStringMatcherPrivate::clearAll()` has sole responsibility for freeing any dynamically allocated memory associated with each member of its `AuxDataList` before clearing the list itself. >Also better to provide a copy constructor since we are copying an AuxData later in the code. I assume that you are referring to copying a `TDEStringMatcherPrivate` object? I’m not sure what I would do in this method that isn’t already done by standard C++ copying. >Have you verified there are no memory leaks with the matchEngine in all possible scenarios (exceptions included)? Verified … all possible scenarios … I can’t say **that**. But I have made sure that `clearAll()` is always called before before any return from `setMatchSpecs()`. And inspecting the output of `TSMTRACE` reveals that there is always a corresponding ‘free’ that follows a regex engine ‘allocate’ for all cases that I have tested. >I would (and will before merging) have to verify that as well, but good if you think about it first. I just installed `valgrind` (which I hadn't realized was FOSS). Is that the tool of choice?
fnmatchFlags = FNM_EXTMATCH; // Bash shell option 'extglob'
#endif
matchEngine = nullptr;
patternConverted = "";
Owner

TQString::null is better than ""

`TQString::null` is better than `""`
void TDEStringMatcher::TDEStringMatcherPrivate::clearAll()
{
m_matchSpecString = "";
Owner

TQString::null is better than ""

`TQString::null` is better than `""`
workArea.clearAll();
return false;
}
if ( matchSpec.pattern.find( TQChar(PatterStringDivider) ) >= 0 ) {
Owner

If we make PatterStringDivider a TQChar as explained in another commment, there is no need for additional casting.

If we make `PatterStringDivider` a `TQChar` as explained in another commment, there is no need for additional casting.
case ANCHandling::EQUIVALENCE :
inferredOptionString += TQChar('e');
auxWork.isCaseSensitive = true;
auxWork.patternConverted = TDEGlobal::equivChars()->replaceChars( auxWork.patternConverted, true );
Owner

Becomes TEquivChars::replaceChars(...) if we make the method static.

Becomes `TEquivChars::replaceChars(...)` if we make the method static.
return false;
}
break;
#endif // Otherwise we will test wildcard expression as one converted to a regex
Owner

Better to make this more explicit in the code, because if someone misses the comment, he can think that the wildcard case is ignored if fnmatch is not available.

Better to make this more explicit in the code, because if someone misses the comment, he can think that the wildcard case is ignored if `fnmatch` is not available.
Poster
Collaborator

#endif // Otherwise we will test wildcard expression as one converted to a regex
Better to make this more explicit in the code, because if someone misses the comment, he can think that the wildcard case is ignored if fnmatch is not available.

I thought it was pretty clear (earlier in the code) how wildcard expressions are handled without glibc fnmatch but I will try to clarify with an empty #else block containing the comment

>#endif // Otherwise we will test wildcard expression as one converted to a regex >Better to make this more explicit in the code, because if someone misses the comment, he can think that the wildcard case is ignored if fnmatch is not available. I thought it was pretty clear (earlier in the code) how wildcard expressions are handled without glibc `fnmatch` but I will try to clarify with an empty `#else` block containing the comment
#define TSMTRACE kdWarning() << "<TSMTRACE> "
namespace TSM
Owner

How about moving the members in this namespace inside the definition of the TDEStringMatcher class? This avoid the namespace (TSM is not a great name for it) and make clear that these members are logically related to TDEStringMatcher.

How about moving the members in this namespace inside the definition of the `TDEStringMatcher` class? This avoid the namespace (`TSM` is not a great name for it) and make clear that these members are logically related to `TDEStringMatcher`.
Poster
Collaborator

How about moving the members in this namespace inside the definition of the TDEStringMatcher class? This avoid the namespace (TSM is not a great name for it) and make clear that these members are logically related to TDEStringMatcher.

I was trying to make constant names shorter but now realize that using this namespace doesn’t really save that much. If only I had called the class TSM instead of TDEStringMatcher in the first place :)

>How about moving the members in this namespace inside the definition of the `TDEStringMatcher` class? This avoid the namespace (`TSM` is not a great name for it) and make clear that these members are logically related to `TDEStringMatcher`. I was trying to make constant names shorter but now realize that using this namespace doesn’t really save that much. If only I had called the class `TSM` instead of `TDEStringMatcher` in the first place :)
typedef TQValueVector<MatchSpec> MatchSpecList;
// Use horizontal tab as m_patternString separator
inline constexpr char PatterStringDivider { '\t' };
Owner

This can be a static member of TDEStringMatcher if moved into the class definition. It would be better to use a TQChar instead of char. Also why using braces for settings its value?

This can be a static member of `TDEStringMatcher` if moved into the class definition. It would be better to use a `TQChar` instead of `char`. Also why using braces for settings its value?
Poster
Collaborator

inline constexpr char PatterStringDivider { '\t' };
This can be a static member of TDEStringMatcher if moved into the class definition. It would be better to use a TQChar instead of char. Also why using braces for settings its value?

I generally prefer to declare/define named literal compile-time constants in headers. Even if I used a TQChar variable instead, I would still want to use the named constant to initialize it.

The braces were mistakenly propagated from an earlier attempt at declaring an array this way.

>`inline constexpr char PatterStringDivider { '\t' };` >This can be a static member of `TDEStringMatcher` if moved into the class definition. It would be better to use a `TQChar` instead of char. Also why using braces for settings its value? I generally prefer to declare/define named literal compile-time constants in headers. Even if I used a `TQChar` variable instead, I would still want to use the named constant to initialize it. The braces were mistakenly propagated from an earlier attempt at declaring an array this way.
@return a basic regular expression formed by converting the basic
wildcard expression in @param wildcardPattern.
*/
TQString wildcardToRegex( const TQString& wildcardPattern );
Owner

Consider making it a protected method.

Consider making it a protected method.
expression characters escaped. Useful for regular expression engines
that do not support /Q.../E.
*/
TQString escapeRegexChars( const TQString& basicString );
Owner

Consider making it a protected method.

Consider making it a protected method.
//typedef wchar_t CHAR16;
//typedef unsigned short CHAR16;
typedef TQChar CHAR16;
Owner

If you are now ok with the usage of TQChar (see other comment), then the typedef for CHAR16 becomes superfluous. We can simply use TQChar in its place.

If you are now ok with the usage of TQChar (see other comment), then the typedef for `CHAR16` becomes superfluous. We can simply use `TQChar` in its place.
//typedef unsigned short CHAR16;
typedef TQChar CHAR16;
class TEquivChars_Private
Owner

class TEquivChars_Private
No longer required, see comments in tdecore/tequivchars.h about making TEquivChars::replaceChars a static method

`class TEquivChars_Private` No longer required, see comments in `tdecore/tequivchars.h` about making `TEquivChars::replaceChars` a static method
};
const defaultCollation EquivalentsTable // terminating ';' is provided in include file
#include "tequivchars-mapping.h"
Owner

EquivalentsTable can be a static const member.
Still I don't understand the point of splitting the definition statement across two files, making reading the code more cumbersome without much benefit.

`EquivalentsTable` can be a static const member. Still I don't understand the point of splitting the definition statement across two files, making reading the code more cumbersome without much benefit.
Poster
Collaborator

Still I don't understand the point of splitting the definition statement across two files, making reading the code more cumbersome without much benefit.

The benefit for me is to make tequivchars.cpp easier to view and edit without needing to wade through the clutter of ~3000 lines of integer constant pairs.

>Still I don't understand the point of splitting the definition statement across two files, making reading the code more cumbersome without much benefit. The benefit for me is to make `tequivchars.cpp` easier to view and edit without needing to wade through the clutter of ~3000 lines of integer constant pairs.
CHAR16 prevChar = 0;
CHAR16 nextChar = 0;
for ( int i = 0 ; i < inStrLen ; outString[i] = CHAR16(currChar), i++ ) {
Owner

currChar is already a CHAR16 so there is no need for explicit casting.

`currChar` is already a `CHAR16` so there is no need for explicit casting.
if ( inBraceExpr ) {
// Is it time to leave brace expression {___} ?
if ( nextChar == '}' ) inBraceExpr = true;
Owner

Should it be false instead of true?

Should it be `false` instead of `true`?
Poster
Collaborator

Should it be false instead of true?

Oops, yes it should!

> Should it be `false` instead of `true`? Oops, yes it should!
// We can process ASCII quickly without using lookup table
unsigned short codepoint = currChar.unicode();
if ( codepoint < 128 ) {
if ( codepoint > 64 && codepoint < 91 ) // convert upper case ASCII
Owner

TQChar::lower() does the same job I believe.

`TQChar::lower()` does the same job I believe.
Poster
Collaborator

TQChar::lower() does the same job I believe.

That is true but I think my method is faster due to the fact TQChar::lower() is doing its own unicode table lookup and table lookups are what I am trying to avoid. But I will grant that it's probably a negligible difference.

>`TQChar::lower()` does the same job I believe. That is true but I think my method is faster due to the fact `TQChar::lower()` is doing its own unicode table lookup and table lookups are what I am trying to avoid. But I will grant that it's probably a negligible difference.
/* FIXME: Possible ideas for optimizing table lookup speed
(1) Detect & handle ASCII (<128) characters separately. *DONE*
(2) Split table into multiple lookup tables and search each
Owner

Binary search is very efficient. Splitting and searching across multiple tables is likely to be slower.

Binary search is very efficient. Splitting and searching across multiple tables is likely to be slower.
expression and the alphabetical characters inside Posix bracket [::]
expressions are left as-is
*/
TQString replaceChars( const TQString &inputString, bool isRegex = false );
Owner

Having now seen how this is used in the code, this method can be a static method and EquivalentsTable can be a static const array. This save the needs for having a equivChars() method and related object in TDEGlobal, which seems unnecessary since the code in this function does not requires any memory between executions.

Having now seen how this is used in the code, this method can be a `static` method and `EquivalentsTable` can be a static const array. This save the needs for having a `equivChars()` method and related object in `TDEGlobal`, which seems unnecessary since the code in this function does not requires any memory between executions.
Poster
Collaborator

Having now seen how this is used in the code, this method can be a static method and EquivalentsTable can be a static const array. This save the needs for having a equivChars() method and related object in TDEGlobal, which seems unnecessary since the code in this function does not requires any memory between executions.

My main goal was to ensure that there is only a single copy of that table regardless of how many times the class is used or from where. I am hoping to enhance and use this same class to implement a future listview sort type. Next iteration will have your suggestion implemented.

>Having now seen how this is used in the code, this method can be a static method and `EquivalentsTable` can be a static const array. This save the needs for having a `equivChars()` method and related object in `TDEGlobal`, which seems unnecessary since the code in this function does not requires any memory between executions. My main goal was to ensure that there is only a single copy of that table regardless of how many times the class is used or from where. I am hoping to enhance and use this same class to implement a future `listview` sort type. Next iteration will have your suggestion implemented.
//-Debug: TSMTRACE << " Attempting to disconnect slots from hidden file matcher (" << m_pHiddenFileMatcher << ") signals ... " << endl;
if (m_pHiddenFileMatcher != nullptr)
disconnect( m_pHiddenFileMatcher, 0, this, 0 );
; //-Debug: TSMTRACE << " ... all slots successfully disconnected" << endl;
Owner

; //-Debug: TSMTRACE << "

The debug statement is commented out but note that it would not be part of the if body if it was uncommented, due to missing braces.

`; //-Debug: TSMTRACE << "` The debug statement is commented out but note that it would not be part of the `if` body if it was uncommented, due to missing braces.
if ( hiddenFileMatcher != nullptr ) {
//-Debug: TSMTRACE << " New pattern string: " << hiddenFileMatcher->getMatchSpecString() << endl ;
//-Debug: TSMTRACE << " Attempting to reconnect slots to hidden file matcher signals ... " << endl;
if ( connect( m_pHiddenFileMatcher, TQT_SIGNAL( destroyed() ), this, TQT_SLOT( setHiddenFileMatcher() ) ) )
Owner

if ( connect(

I understand the if was used here for debugging purposes and it is fine.
For the final version (without TSM tracing), please remove the if around the connect

`if ( connect( ` I understand the `if` was used here for debugging purposes and it is fine. For the final version (without TSM tracing), please remove the `if` around the `connect`
Poster
Collaborator

if ( connect(
I understand the if was used here for debugging purposes and it is fine.
For the final version (without TSM tracing), please remove the if around the connect

I did that and have removed all TSMTRACE statements from this particular file

>if ( connect( >I understand the if was used here for debugging purposes and it is fine. For the final version (without TSM tracing), please remove the if around the connect I did that and have removed all `TSMTRACE` statements from this particular file
Poster
Collaborator

Despite a possible appearance to the contrary, I have NOT abandoned this effort! I had to turn my attention to other matters over the summer and those took a lot longer than expected. I am now back on the job :)

Despite a possible appearance to the contrary, I have NOT abandoned this effort! I had to turn my attention to other matters over the summer and those took a lot longer than expected. I am now back on the job :)
Poster
Collaborator

I have reviewed the feedback from @MicheleC and have made (for the most part) the recommended changes. I will push the updated code after rebasing. I will also upload the remaining changes to tdelibs that are necessary to address the original issue.

I have reviewed the feedback from @MicheleC and have made (for the most part) the recommended changes. I will push the updated code after rebasing. I will also upload the remaining changes to `tdelibs` that are necessary to address the original issue.
VinceR force-pushed issue/270/tdelibs-V4 from f0cff95fb1 to 683d212303 7 months ago
Owner

Hi @VinceR,
thanks for the update on this PR. I will review the changes after the release of R14.1.1, which is planned at the end of this month.

Hi @VinceR, thanks for the update on this PR. I will review the changes after the release of R14.1.1, which is planned at the end of this month.

Reviewers

MicheleC requested changes 10 months ago
This pull request is marked as a work in progress.
Sign in to join this conversation.
No reviewers
No Milestone
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: TDE/tdelibs#209
Loading…
There is no content yet.