Trim Quotes from start, & end of log file #1508
Conversation
| logFileName = Guid.NewGuid().ToString(); | ||
| } | ||
|
|
||
| LogFile = Path.GetTempPath() + logFileName + ".TpTrace.log"; |
There was a problem hiding this comment.
Should we use Path.Combine here?
There was a problem hiding this comment.
it's all same internally
There was a problem hiding this comment.
Path.GetTempPath() value depends on Env variable. User may set it to C:\Temp or C:\Temp.
There was a problem hiding this comment.
the behavior for UWP is different, all BCL have implementation according to CLR, UWP apps cannot write in C:\Temp, they are allocated their own temp folder inside app local data.
There was a problem hiding this comment.
C:\Temp was just an example I gave. In UWP how the Path.GetTempPath() get determined? User can modify that? If so can he ignore \ at end?
There was a problem hiding this comment.
no idea how it is overwritten.
There was a problem hiding this comment.
Then we better use Path.Combine().
There was a problem hiding this comment.
lets use Path.Combine. There is a reason why Path.Combine is there in BCL and we shouldn't re implement it
| } | ||
| catch | ||
| { | ||
| logFileName = Guid.NewGuid().ToString(); |
There was a problem hiding this comment.
Should we not throw error here as this is possible only if customLogFile path has invalid chars. Should we write logs in new file name?
There was a problem hiding this comment.
We want to disable logging because file name was incorrect, which we constructed?
There was a problem hiding this comment.
Does this log file path not coming as diag arg value?
There was a problem hiding this comment.
yes, but we also add bunch of data to it to make it unique.
There was a problem hiding this comment.
ok. got it.
So in case user passed some invalid chars in logFilePath, and in result we are creating a log file with different name, is that ok with us?
There was a problem hiding this comment.
What if user passed valid file name, & we corrupted it?
There was a problem hiding this comment.
Yes, got that case. Just pointing that there is a scenario in which user given file path will not be respected and user will not be aware of the same (as we will change the log file name and not inform the user)
There was a problem hiding this comment.
We cannot inform user of that change, this is the first step that we are doing while launching App.
Also your logging should not be based on file name, but unique extension.
Add try catch as Path.GetFileNameWithoutExtension() can throw exception. If exception is thrown application hangs, so avoiding that.