Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DR2 2047 allow modification of max chars per cell #532

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

techncl
Copy link
Contributor

@techncl techncl commented Jan 29, 2025

No description provided.

def getColumnFromCsv(csvFile: TextFile, csvSchemaFile: TextFile, columnName: String): List[String] = Try {
val validator = createValidator(true, Nil, false, false, false)
val csv = validator.loadCsvFile(csvFile, csvSchemaFile)
def getColumnFromCsv(csvFile: TextFile, csvSchemaFile: TextFile, columnName: String, maxCharsPerCell: Int): List[String] = Try {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have to send the maxCharsPerCell to the validator as well as when calling a method on the validator?
If the validator knows about it, the method loadCsvFile can rely on using this.maxCharsPerCell

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yes! Good point. I think I was just trying to get it to work initially, and didn't go back to clean up.

}

private def validate(csvFile: String, csvEncoding: Charset, validateCsvEncoding: Boolean, csvSchemaFile: String, csvSchemaEncoding: Charset, validateCsvSchemaEncoding: Boolean, failFast: Boolean, pathSubstitutionsList: JList[Substitution], enforceCaseSensitivePathChecks: Boolean, trace: Boolean, progress: Option[SProgressCallback], skipFileChecks: Boolean): JList[FailMessage] = {
private def validate(csvFile: String, csvEncoding: Charset, validateCsvEncoding: Boolean, csvSchemaFile: String, csvSchemaEncoding: Charset, validateCsvSchemaEncoding: Boolean, failFast: Boolean, pathSubstitutionsList: JList[Substitution], enforceCaseSensitivePathChecks: Boolean, trace: Boolean, progress: Option[SProgressCallback], skipFileChecks: Boolean, maxCharsPerCellLimit: Int): JList[FailMessage] = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we give the default value of 4096 here, so as to avoid the callers sending in 4096 as it looks like a magic number

toConsole("Error: Maximum number of errors to display should be more than 0")
0
}
val maxCharsPerCell = convertTextboxValueToInt(potentialMaxCharsPerCell, "errors to display", toConsole)
Copy link
Contributor

@sparkhi sparkhi Jan 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error messages are swapped for textboxes

Copy link
Contributor

@sparkhi sparkhi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine a couple of small changes.
And one thing to consider was about passing in "maxCharsPerCell" to validator class as well as methods on it and it seem to be in many places. I was thinking it should only be set on validator.

@techncl techncl force-pushed the DR2-2047_allowModificationOfMaxCharsPerCell branch from 27dba83 to 96ff697 Compare January 31, 2025 17:15
@techncl techncl force-pushed the DR2-2047_allowModificationOfMaxCharsPerCell branch 3 times, most recently from dfc925c to f5f44d9 Compare February 4, 2025 10:00
@techncl techncl force-pushed the DR2-2047_allowModificationOfMaxCharsPerCell branch from f5f44d9 to 7ff37ba Compare February 4, 2025 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants