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

Provide variable for current selection #1

Open
rchl opened this issue Dec 19, 2020 · 6 comments
Open

Provide variable for current selection #1

rchl opened this issue Dec 19, 2020 · 6 comments

Comments

@rchl
Copy link

rchl commented Dec 19, 2020

I was checking your code to see if I could use it to replace my clang-format plugin [1] and found that my plugin also supports formatting just the selection. Or at least passing the current selection to clang-format [2].

Maybe you could provide variables that would expand to selection_start, selection_end offsets, and also selection_length?

(Although, I've just realized while writing this, that we wouldn't want to pass -offset and -length when the selection is empty so that could be another thing that would make supporting this tricky...)

[1] https://github.com/rchl/SublimeClangFormat
[2] https://github.com/rchl/SublimeClangFormat/blob/9d4f804e82b471928fab0c347ccabefccaf9240c/ClangFormat.py#L128-L132

(Also, I would really want to run formatting on a separate thread so that the UI doesn't block when formatting MB-big files).

@mitranim
Copy link
Owner

Interesting.

Adding a command or argument to format each selected region, rather than buffer, should be pretty simple. The plugin could use the scopes at selection start, rather than buffer start, to determine which formatter to use (more precisely, from which "rule" to take the settings, which is basically the same). Would be interesting to hear about use cases. Can see this being useful for stuff like JS-in-HTML, but there's probably a lot more.

Unsure how to handle indentation when formatting only selected regions. Examples would help.

Folks in the ST Discord also mentioned async formatting. The general conclusion was that sometimes you want sync behavior (gofmt, clang-format on actual source code, etc.), and sometimes async behavior (multi-MB bundles). I'm considering adding a setting for this. Just unsure how to handle conflicts. Not blocking the UI for the duration of the formatting process allows you to edit this code before it gets replaced, which means you'll be losing changes. If you weren't going to edit it, I'm not entirely sure why bother making it async, unless formatting takes a minute and you'd like to switch to another tab in the meantime. Would be useful to hear about expected / desired behavior. 🙂

@rchl
Copy link
Author

rchl commented Dec 20, 2020

Adding a command or argument to format each selected region, rather than buffer, should be pretty simple. The plugin could use the scopes at selection start, rather than buffer start, to determine which formatter to use (more precisely, from which "rule" to take the settings, which is basically the same). Would be interesting to hear about use cases. Can see this being useful for stuff like JS-in-HTML, but there's probably a lot more.

That solution sounds good to me (having two separate configurations/commands).

As for clang-format, I believe that when you are formatting just the selection (by providing the -offset and -length, you are still getting passing and getting the whole buffer. It's just the formatter will attempt to format only a given range (which is gonna be just interpreted roughly as it's not always possible to format just the selection and the formatter might expand that to the whole statement, for example).

The most common use-case is probably to just format the current line rather than the whole document.

Unsure how to handle indentation when formatting only selected regions. Examples would help.

In the case of clang-format I think that no special handling would be necessary since the whole buffer would be returned regardless if formatting just the selection or the whole document.

Folks in the ST Discord also mentioned async formatting. The general conclusion was that sometimes you want sync behavior (gofmt, clang-format on actual source code, etc.), and sometimes async behavior (multi-MB bundles)

To be fair, IMO the async formatting would always be preferred as even with very fast formatters, you'd likely see a tiny stutter on saving if you are sensitive to that.

I know that that makes the on_pre_save handler useless though so an option is probably the best "option".

Just unsure how to handle conflicts.

You could just cancel the formatting if the document was modified since triggering the formatting. This can be checked using view.change_count(). If the user gets a clear message (maybe just in the status bar) that the formatting was canceled then it should be fine for those rare cases where the user changes something during formatting.

@mitranim
Copy link
Owner

In a general case we have N selection regions. Does clang-format support formatting N selections, by passing offset-length multiple times?

When files are being watched, you don't want to save twice, as it would trigger a redundant rebuild/rerun. A good watcher will be able to cancel and restart (gow does), but a bad one will just build twice, wasting your CPU/battery and time (gulp/webpack). This makes sync on_pre_save formatting essential for such scenarios. There might be other ways to reliably prevent double save, but I'm not aware of them. This means async would have to be an option. Both approaches have edge cases they don't handle well, so if there's something better, I'd love to know about it.

Canceling format on change is an interesting idea, thanks.

@rchl
Copy link
Author

rchl commented Dec 21, 2020

Does clang-format support formatting N selections, by passing offset-length multiple times?

Yes. You can read about it more at https://clang.llvm.org/docs/ClangFormat.html

@mitranim
Copy link
Owner

It would be possible with variables if clang-format supported passing them like this:

-offset 10,20,30 -length 40,50,60

The documentation seems to indicate that you need pairs:

-offset 10 -length 40 -offset 20 -length 50 -offset 30 -length 60

If true, I'm slightly at a loss how to support the latter.

@mitranim
Copy link
Owner

We could have variables for "start of first selection" and "length of first selection". There could be incompatibilities between how ST and clang-format count the characters, in which case we might need more variables to support that too.

The user might want separate hotkeys and separate Fmt rules. For example, when formatting a selection, the user would want to activate a rule which contains offset and length for that selection, but without a selection, they want a rule without those flags. This implies the ability to choose between different rules for the same scope. Currently Fmt looks up the rule by scope, assuming just 1 way to format any given scope. Suggestions would be appreciated.

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

No branches or pull requests

2 participants