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

[main] command-line argument +a:b is parsed as row:col, contradicting man page #2357

Closed
midichef opened this issue Mar 23, 2024 · 8 comments · Fixed by #2358
Closed

[main] command-line argument +a:b is parsed as row:col, contradicting man page #2357

midichef opened this issue Mar 23, 2024 · 8 comments · Fixed by #2358
Labels

Comments

@midichef
Copy link
Contributor

Small description
vd sample_data/benchmark.csv +:0:2 opens the sheet with the cursor at the third column.

Expected result
The documentation suggests it should open at the third row, not column:

vd [options] [input ...] +toplevel:subsheet:col:row

What's the correct order here: +col:row or +row:col? I've got a PR to fix some bugs in handling the initial row/columns passed in the +a:b argument. Should I match the man page and docstrings which say col:row? Or the existing behavior and the tests in features/slide.py, which do row:col?

Additional context
saul.pw/VisiData v3.1dev

@midichef midichef added the bug label Mar 23, 2024
@anjakefala
Copy link
Collaborator

@aborruso, this feature is your request! Can you please tell midichef your preferences for its syntax? =)

@anjakefala
Copy link
Collaborator

(Nice catch, midichef)

@aborruso
Copy link
Contributor

Tomorrow I try to remember what the request was and then I answer you. Thank you very much

1 similar comment
@aborruso
Copy link
Contributor

Tomorrow I try to remember what the request was and then I answer you. Thank you very much

@anjakefala
Copy link
Collaborator

anjakefala commented Mar 23, 2024

When I went to look up the original request, it seems like you cared about selecting the top-level sheet from the CLI! #214 (comment) Not selecting a specific row/column. It wasn't you who originally proposed the orderl. =)

Saul designed it originally row:col. I think he chose that because VisiData is row-oriented, and that makes sense to me. I possibly accidentally wrote the documentation as col:row because I often think in a column-oriented fashion. Since VisiData is row-oriented, I vote we keep the existing behaviour, and fix the documentation. I can go ahead and fix it.

@anjakefala
Copy link
Collaborator

I thought about it some more.

i think probably keeping it row:col is the default approach, because we might silently break folks CLI usage with an abrupt change.

But since you did start this conversation midichef, I would love to hear yours, aborruso's and anyone else's preferences. Personally col:row felt more intuitive to me. If this is true for lots of other folks, it possibly makes sense to change it.

@midichef
Copy link
Contributor Author

For a single argument, +row makes sense. It matches the behavior of other programs, like vim +3 and tail +3 (for tail in GNU coreutils). Which is probably where it came from.

For the case of two arguments, +a:b, to me the more intuitive meaning is col:row. It matches the general pattern of geometry for coordinates being given in the order (x,y). For example, that's how vim specifies geometry:

gvim -geometry 80x63+8+100
This creates a window with 80 columns and 63 lines at position 8 pixels from
the left and 100 pixels from the top of the screen.

I agree we should think it over, as it's definitely possible that changing tocol:row may break some scripts that rely on +:row:col. But my guess is that hardly anyone has been using it. Because nobody has complained that the simpler argument pattern +row:col (with only one colon, not two) has been broken for quite some time. It worked on visidata 2.8, but not in 2.9 and after. (At least, on my system.) That simpler form vd +2:2 currently sets the row, but not the column, because the column movement command is applied before the sheet loads. To actually apply a starting column, you have to add an extra : after the +, that is, +:row:col.

I've got a set of commits to get the +a:b syntax working, and fix a few other details, once we make a decision here. (develop...midichef:581cdf12b28278aea87ce09f7c32bf8ae1be2f0f:10e532f41225bfa75d3c9bf527812bc308cd7705)

@aborruso
Copy link
Contributor

Thank you @midichef and @anjakefala. It is a command that I have tried to use without much luck and have removed from the daily commands.
I don't have a specific preference, but I prefer a little bit more col:row, as is the case in a spreadsheet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants