CSV Renderer may guess a TSV file is comma delimited even when tab delimited #7630

@lunny commented on GitHub (Jul 27, 2021):

So I think that's a bug of the function guesses the delimiter.

@lunny commented on GitHub (Jul 27, 2021): So I think that's a bug of the function guesses the delimiter.

GiteaMirror commented

@silverwind commented on GitHub (Jul 27, 2021):

make a separate markup renderer like the "csv" one but call it "tsv" and always use \t as the delimiter due to the filename

I would not make a separate renderer but simply skip over guessDelimiter in case the extension provides a strong indication of the delimiter in use. That way, we can extend with more similar extensions later.

One such extension may be .psv for Pipe Separated Values (|).

@silverwind commented on GitHub (Jul 27, 2021): > make a separate markup renderer like the "csv" one but call it "tsv" and always use \t as the delimiter due to the filename I would not make a separate renderer but simply skip over `guessDelimiter` in case the extension provides a strong indication of the delimiter in use. That way, we can extend with more similar extensions later. One such extension may be `.psv` for Pipe Separated Values (`|`).

GiteaMirror commented

@lunny commented on GitHub (Jul 27, 2021):

Hm, We should allow external renderer has their own parameters in app.ini.

@lunny commented on GitHub (Jul 27, 2021): Hm, We should allow external renderer has their own parameters in `app.ini`.

GiteaMirror commented

@richmahn commented on GitHub (Jul 27, 2021):

Yes, we need to have the filename available so that it can be use in determining which delimiter to use as well.

@richmahn commented on GitHub (Jul 27, 2021): Yes, we need to have the filename available so that it can be use in determining which delimiter to use as well.

GiteaMirror commented

2025-11-02 07:31:53 -06:00

@richmahn commented on GitHub (Jul 27, 2021):

make a separate markup renderer like the "csv" one but call it "tsv" and always use \t as the delimiter due to the filename

I would not make a separate renderer but simply skip over guessDelimiter in case the extension provides a strong indication of the delimiter in use. That way, we can extend with more similar extensions later.

One such extension may be .psv for Pipe Separated Values (|).

Yes, but the problem is currently there is no extension passed to the renderer, as it is invoked by the list of extensions it renders. I offered that as a solution though, if we can get it passed in.

@richmahn commented on GitHub (Jul 27, 2021): > > make a separate markup renderer like the "csv" one but call it "tsv" and always use \t as the delimiter due to the filename > > I would not make a separate renderer but simply skip over `guessDelimiter` in case the extension provides a strong indication of the delimiter in use. That way, we can extend with more similar extensions later. > > One such extension may be `.psv` for Pipe Separated Values (`|`). Yes, but the problem is currently there is no extension passed to the renderer, as it is invoked by the list of extensions it renders. I offered that as a solution though, if we can get it passed in.

GiteaMirror commented

@richmahn commented on GitHub (Jul 27, 2021):

Hm, We should allow external renderer has their own parameters in app.ini.

@lunny What kind of parameters? You mean what delimiter to use if it is a tsv, a csv, or a psv? Still csv is actually the only standard, which can be any character delimited (some say csv means character-separated-values), so one repo might use it different than another. Yet tsv and psv are csv files yet kind of hint at the delimiter in their extension.

@richmahn commented on GitHub (Jul 27, 2021): > Hm, We should allow external renderer has their own parameters in `app.ini`. @lunny What kind of parameters? You mean what delimiter to use if it is a tsv, a csv, or a psv? Still csv is actually the only standard, which can be any character delimited (some say csv means character-separated-values), so one repo might use it different than another. Yet tsv and psv are csv files yet kind of hint at the delimiter in their extension.

GiteaMirror commented

2025-11-02 07:31:53 -06:00

@richmahn commented on GitHub (Jul 27, 2021):

From a coworker:

The history of this puzzles me a little @Rich Mahn. If I was writing code to autodetect TSV vs CSV, I'd look at the first/header line. And that line only has the column headers and tabs and zero commas. Why would it need to look further? How many lines was the old code scanning?

I do wonder if the first row, whether a header or not, is enough to go by, rather than counting all tabs and commas and other delimiters ALL the content as it currently does. You'd think that would be using more tabs than commas, or commas than tabs especially if it is a real header. But even if not, seems like still the best way to score the characters.

Edit: I see now it scores based on the first 10 lines, yet as my example has, my 2nd row actually has a bunch of commas in the text. Headers wouldn't normally do that.

@richmahn commented on GitHub (Jul 27, 2021): From a coworker: > The history of this puzzles me a little @Rich Mahn. If I was writing code to autodetect TSV vs CSV, I'd look at the first/header line. And that line only has the column headers and tabs and zero commas. Why would it need to look further? How many lines was the old code scanning? I do wonder if the first row, whether a header or not, is enough to go by, rather than counting all tabs and commas and other delimiters ALL the content as it currently does. You'd think that would be using more tabs than commas, or commas than tabs especially if it is a real header. But even if not, seems like still the best way to score the characters. Edit: I see now it scores based on the first 10 lines, yet as my example has, my 2nd row actually has a bunch of commas in the text. Headers wouldn't normally do that.

GiteaMirror commented

2025-11-02 07:31:53 -06:00

@richmahn commented on GitHub (Jul 27, 2021):

@lunny @silverwind

I suggest two things:

get the file extension passed to the Renderer so it can go by that if it isn't just .csv
When it is .csv, to guess the delimiter, we could do it from the first 10k or 10 rows, but why not then quickly see if the top scoring one matches with all 10 rows with the same number of fields, if not, the 2nd, 3rd, or 4th (assuming score > 0) and use the one that works, and if none do, return the top scoring one. If all have no score, then return , (comma). (how well it matches each row can be part of the score)

Thoughts?

@richmahn commented on GitHub (Jul 27, 2021): @lunny @silverwind I suggest two things: 1) get the file extension passed to the Renderer so it can go by that if it isn't just .csv 2) When it is .csv, to guess the delimiter, we could do it from the first 10k or 10 rows, but why not then quickly see if the top scoring one matches with all 10 rows with the same number of fields, if not, the 2nd, 3rd, or 4th (assuming score > 0) and use the one that works, and if none do, return the top scoring one. If all have no score, then return `,` (comma). (how well it matches each row can be part of the score) Thoughts?

GiteaMirror commented

@lunny commented on GitHub (Jul 28, 2021):

I agree both. We can read ten lines to detect the delimiter. But one we need notice that some column have been quoted with double quote and contains delimiter characters in the double quote. i.e.

a,b,"c,b,d,",e

It should be 4 columns but not 7. If we just simple count single quote, that will be wrong.

@lunny commented on GitHub (Jul 28, 2021): I agree both. We can read ten lines to detect the delimiter. But one we need notice that some column have been quoted with double quote and contains delimiter characters in the double quote. i.e. ``` a,b,"c,b,d,",e ``` It should be 4 columns but not 7. If we just simple count single quote, that will be wrong.

GiteaMirror commented

@richmahn commented on GitHub (Jul 28, 2021):

@lunny yeah, I thought of that and well aware of that. I assume to actually use the encoding/cvs module with those 10 lines and the delimiter in question and see what it returns. We'd still be making a best guess, as some rows might have the wrong number of fields, but if it has a higher accuracy than other delimiters, will use that delimiter. That is why we can kind of do a score with the matching of the delimiter as well.

@richmahn commented on GitHub (Jul 28, 2021): @lunny yeah, I thought of that and well aware of that. I assume to actually use the encoding/cvs module with those 10 lines and the delimiter in question and see what it returns. We'd still be making a best guess, as some rows might have the wrong number of fields, but if it has a higher accuracy than other delimiters, will use that delimiter. That is why we can kind of do a score with the matching of the delimiter as well.

GiteaMirror commented

@richmahn commented on GitHub (Jul 29, 2021):

So just to be clear, making a fix for this that should also work with the CSV diff feature which has the same bug.

@richmahn commented on GitHub (Jul 29, 2021): So just to be clear, making a fix for this that should also work with the CSV diff feature which has the same bug.

GiteaMirror commented

@zeripath commented on GitHub (Jul 31, 2021):

Why don't we provide a mechanism for choosing whether to guess the delimiter or not.

@zeripath commented on GitHub (Jul 31, 2021): Why don't we provide a mechanism for choosing whether to guess the delimiter or not.

GiteaMirror commented

@richmahn commented on GitHub (Aug 5, 2021):

@zeripath So some sort of user interface the repo file view page? Yet would still have to guess something when first showing the file.

@richmahn commented on GitHub (Aug 5, 2021): @zeripath So some sort of user interface the repo file view page? Yet would still have to guess something when first showing the file.

GiteaMirror commented

@richmahn commented on GitHub (Aug 5, 2021):

@zeripath Also, the DIFF view for .csv files has this bug, and thus you'd have to have a selection for each file diff.

@richmahn commented on GitHub (Aug 5, 2021): @zeripath Also, the DIFF view for .csv files has this bug, and thus you'd have to have a selection for each file diff.

GiteaMirror commented

@silverwind commented on GitHub (Aug 5, 2021):

Why don't we provide a mechanism for choosing whether to guess the delimiter or not.

I wouldn't. This is a kind of thing that can be handled automatically and should not need configuration for the most common types (csv,tsv,psv).

https://github.com/go-gitea/gitea/issues/16558#issuecomment-887807325 sounds good to me.

@silverwind commented on GitHub (Aug 5, 2021): > Why don't we provide a mechanism for choosing whether to guess the delimiter or not. I wouldn't. This is a kind of thing that can be handled automatically and should not need configuration for the most common types (csv,tsv,psv). https://github.com/go-gitea/gitea/issues/16558#issuecomment-887807325 sounds good to me.

GiteaMirror commented

@zeripath commented on GitHub (Aug 5, 2021):

Every heuristic mechanism will get things wrong for some pathological case.

@zeripath commented on GitHub (Aug 5, 2021): Every heuristic mechanism will get things wrong for some pathological case.

GiteaMirror commented