Skip to content

Add command to toggle CSV column aligning #45

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.

Sign up for GitHub

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

Open
mkruselj wants to merge 1 commit into
base: master
Choose a base branch
from
+61 −6
Open

Add command to toggle CSV column aligning #45

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions Context.sublime-menu
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@
{ "command": "disable", "caption": "Disable" },
{ "command": "run_query", "caption": "Run RBQL query (F5)" },
{ "command": "set_table_name", "caption": "Set table name for RBQL" },
{ "command": "align", "caption": "Align (with trailing spaces)" },
Copy link
Copy Markdown
Owner

@mechatroner mechatroner Oct 15, 2025

Choose a reason for hiding this comment

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

Let's keep the original commands.

Copy link
Copy Markdown
Author

@mkruselj mkruselj Oct 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Kind of doesn't make sense when this one is arguably better? I consider it information overload, you have three actions that are all tied to the exact same thing, and two of them are basically redundant...

Copy link
Copy Markdown
Owner

@mechatroner mechatroner Oct 16, 2025

Choose a reason for hiding this comment

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

To your point, I would actually keep the 2 original commands and remove the toggle command, it is nice to have it as a button (but there is no way to customize UI in Sublime AFAIK) or as a command.

Copy link
Copy Markdown
Author

@mkruselj mkruselj Oct 16, 2025

Choose a reason for hiding this comment

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

I really don't see why it's such a problem to have a neat single action instead of two clunky ones... I may be biased but this single action is simply better UX regardless if it's on a button in the GUI, if it's a keybind, or if it's a menu entry...

Copy link
Copy Markdown
Owner

@mechatroner mechatroner Oct 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Here is my thinking: It does make sense to have a single keybinding to the toggle command, because it is harder to remember 2 keybindings than one. User mental memory is important. So it is fine to make the command less precise, but the upside is we save the user from the headache. Same goes for the UI button in the front screen - the screen area is limited so we only can afford to have 1 less precise button. Totally different situation with the context menu, it only shown in rare situations and we want to be precise there, I don't mind to have 2 buttons in the context menu and IMO it is more convenient to have it this way. WDYT?

Copy link
Copy Markdown
Author

@mkruselj mkruselj Oct 16, 2025

Choose a reason for hiding this comment

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

I am not sure if it's really more convenient to have two separate context menu entries that both deal with the exact same thing. This is the best possible case for checkmarked single entries. Less cognitive load (two separate context menu entries have different wording, here you have one single wording and a checkmark to tell you it's on - easy!)

😅

{ "command": "shrink", "caption": "Shrink (trim spaces from fields)" },
{ "command": "toggle_csv_alignment", "caption": "Align CSV columns with spaces", "checked": true },
Copy link
Copy Markdown
Owner

@mechatroner mechatroner Oct 15, 2025

Choose a reason for hiding this comment

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

what does "checked": true mean?

Copy link
Copy Markdown
Author

@mkruselj mkruselj Oct 15, 2025

Choose a reason for hiding this comment

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

This enables Sublime Text to run the is_checked() method, which then shows or doesn't show the checkmark next to the action name in the context menu.

Copy link
Copy Markdown
Owner

@mechatroner mechatroner Oct 16, 2025

Choose a reason for hiding this comment

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

Let's just remove the new command from the context menu. Instead, add documentation on how to add hotkey mapping for it, those who need it would do so.

{ "command": "csv_lint", "caption": "CSVLint" },
{ "command": "edit_settings", "caption": "Settings",
"args": {
Expand Down
1 change: 1 addition & 0 deletions Default.sublime-commands
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
{ "caption": "Rainbow CSV: Run RBQL query", "command": "run_query" },
{ "caption": "Rainbow CSV: Align CSV columns with spaces", "command": "align" },
{ "caption": "Rainbow CSV: Shrink CSV table by trimming spaces from fields", "command": "shrink" },
{ "caption": "Rainbow CSV: Toggle aligning CSV columns with spaces", "command": "toggle_csv_alignment" },
{ "caption": "Rainbow CSV: CSVLint", "command": "csv_lint" },
{ "caption": "Rainbow CSV: Enable Simple", "command": "enable_simple" },
{ "caption": "Rainbow CSV: Enable Quoted", "command": "enable_quoted" },
Expand Down
3 changes: 2 additions & 1 deletion Default.sublime-keymap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
[
{"keys": ["f5"], "command": "run_query", "context": [{"key": "setting.rainbow_mode", "operator": "equal", "operand": true}]}
{"keys": ["f5"], "command": "run_query", "context": [{"key": "setting.rainbow_mode", "operator": "equal", "operand": true}]},
{"keys": ["ctrl+alt+a"], "command": "toggle_csv_alignment"}
Copy link
Copy Markdown
Owner

@mechatroner mechatroner Oct 15, 2025

Choose a reason for hiding this comment

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

Let's avoid adding additional key mappings, but instead could you please add a readme instructions how users can create this mapping in their config.

Copy link
Copy Markdown
Author

@mkruselj mkruselj Oct 16, 2025

Choose a reason for hiding this comment

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

Just to give you a perspective on why I really wanted to have a default keybinding here.

As a user, the one thing that annoys me with a lot of Sublime Text extensions is that they don't come with some keybindings to get you started, instead you have to dig around settings files and set up the keybinding manually by writing a JSON incantation. So I thought to actually improve UX for users here, they can just start using the thing straight up, and this align/compact is (after syntax coloring CSVs) the most important feature of this plugin, as far as I'm concerned. It makes things so much easier when editing big CSVs. So being able to do that without any additional setup would have been a huge benefit in my book...

]
60 changes: 57 additions & 3 deletions main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ def make_sublime_settings(syntax_settings_path):

def remove_sublime_settings(syntax_settings_path):
try:
os.remove(syntax_settings_path)
os.remove(syntax_settings_path)
except Exception:
pass

Expand All @@ -364,7 +364,7 @@ def dbg_log(logging_enabled, msg):


def set_syntax_with_timeout(view, syntax_file, logging_enabled, timeout):
# We use this callback with timeout because otherwise Sublime fails to find the brand new .sublime-syntax file right after it's generation -
# We use this callback with timeout because otherwise Sublime fails to find the brand new .sublime-syntax file right after it's generation -
# And shows an error (highlighting would work though, but the error is really ugly and confusing)
# Using workaround suggested here: https://github.com/sublimehq/sublime_text/issues/3477
if timeout > 2000:
Expand Down Expand Up @@ -750,6 +750,60 @@ def run(self, edit):
adjusted_content = '\n'.join(adjusted_lines)
self.view.replace(edit, sublime.Region(0, self.view.size()), adjusted_content)

class ToggleCsvAlignmentCommand(sublime_plugin.TextCommand):

def run(self, edit):
dialect = get_dialect(self.view.settings())
if dialect[1] == 'monocolumn':
sublime.error_message('Error. You need to select a separator first')
return

delim, policy = dialect

# Check if the CSV is currently aligned by examining the content
# We'll look at a few lines to determine the state
is_aligned = self.is_table_aligned(delim, policy)

if is_aligned:
self.view.run_command('shrink')
else:
self.view.run_command('align')

def is_table_aligned(self, delim, policy):
Copy link
Copy Markdown
Owner

@mechatroner mechatroner Oct 15, 2025

Choose a reason for hiding this comment

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

I would prefer to have a global dictionary variable is_aligned_map which maps filenames to their current alignment status. By default, it should be "false" i.e. not aligned.

Copy link
Copy Markdown
Author

@mkruselj mkruselj Oct 15, 2025

Choose a reason for hiding this comment

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

Mmmm wouldn't this potentially create problems if you open the same file in multiple views and set the alignment up differently?

I prefer a slight runtime cost over increasing memory requirement, and this method is sure to work even if the same file is opened in multiple views with different state of alignment...

Copy link
Copy Markdown
Owner

@mechatroner mechatroner Oct 16, 2025

Choose a reason for hiding this comment

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

The runtime check has some issues, e.g. user can edit the file - add/remove whitespaces and it would run the same op again, for different files initial behavior would be different when you map it to the command, since there is no feedback on what to expect. First command invocation should always align. You have a good point about multiple view, I'm actually not sure if it aligns in a single view only, I would appreciate if you check, but if it indeed the case then we have to store the variable in the view state (if there is such a thing) or use view id or something similar. I don't like heuristics and guessing when it is possible to avoid them with explicit state-based logic

"""
Determine if the table is currently aligned by checking if fields
have trailing spaces before delimiters
"""
# Sample a few lines from the document
line_regions = self.view.lines(sublime.Region(0, self.view.size()))
sample_size = min(5, len(line_regions))

aligned_count = 0

for i in range(sample_size):
line = self.view.substr(line_regions[i])
fields, warning = csv_utils.smart_split(line, delim, policy, True)

if warning:
continue

# Check if any field (except the last one) has trailing spaces
for j in range(len(fields) - 1):
if fields[j] != fields[j].rstrip():
aligned_count += 1
break

# If more than half of the sampled lines show alignment, consider it aligned
return aligned_count >= (sample_size / 2)

def is_checked(self):
Comment thread
mkruselj marked this conversation as resolved.
dialect = get_dialect(self.view.settings())
if dialect[1] == 'monocolumn':
return False

delim, policy = dialect
return self.is_table_aligned(delim, policy)


def csv_lint(view, delim, policy):
if policy == 'quoted_rfc':
Expand Down Expand Up @@ -919,7 +973,7 @@ def find_unbalanced_lines_around(view, center_line):
search_range = 10
search_begin = max(0, center_line - search_range)
search_end = min(num_lines, center_line + search_range)
start_line = None
start_line = None
end_line = None
for l in range(search_begin, search_end):
cur_line = get_line_text(view, l)
Expand Down