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

Investigate viability of long-line wrapping #700

Open
Xophmeister opened this issue Apr 9, 2024 · 6 comments
Open

Investigate viability of long-line wrapping #700

Xophmeister opened this issue Apr 9, 2024 · 6 comments

Comments

@Xophmeister
Copy link
Member

Xophmeister commented Apr 9, 2024

This has been mentioned a few times. For example:

  1. There doesn't seem to be a way to have something for max line width. Perhaps leveraging the softline captures to tell the formatter that it's ok to wrap long lines of code across multiple lines without requiring the developer to multiline first?

That's right; Topiary has no concept of line-length and using that as a constraint to force reflowing, like other formatters do. I'm not sure if this is "by design" or "by accident", but the general nature of Topiary means that it's not obvious how to wrap long lines in all cases.

Originally posted in #699 (review)

Per the comment, Topiary currently doesn't do anything with long-lines. It would be worth investigating whether anything suitably general can be done (e.g., forcing multiline mode at a certain column, etc.).

@ErinvanderVeen ErinvanderVeen added the epic: contributability Relates to how easy it is for external contributors to contribute to Topiary (engine and queries) label Apr 9, 2024
@aspiwack
Copy link
Member

aspiwack commented Apr 9, 2024

The difficulty is that as soon as you deal with long lines, then you have an optimisation problem to solve.

If you decide, for instance, that all the forms that cross the maximum column are considered multiline, you'll often make a lot of very short lines. So you need a more complex strategy. It's kind of tricky, this is where pretty printing libraries come in handy. I don't have a clear idea how such pretty printers and our current output strategy would meld.

But it is definitely a feature that some people want.

@catskul
Copy link

catskul commented Aug 10, 2024

So I think there is currently nearly a way of doing this though I haven't quite figured out how to make it work. And it's quite a hack with some obvious limitations. Though someone more attuned to how topiary and tree-sitter work might be able to do this better.

First, arbitrary captures have to be enabled, by removing the error on non-matching capture so that regex matches can be used:

diff --git a/topiary-core/src/atom_collection.rs b/topiary-core/src/atom_collection.rs
index 56d1e00..9738762 100644
--- a/topiary-core/src/atom_collection.rs
+++ b/topiary-core/src/atom_collection.rs
@@ -390,10 +390,10 @@ impl AtomCollection {
             }
             // Return a query parsing error on unknown capture names
             unknown => {
-                return Err(FormatterError::Query(
-                    format!("@{unknown} is not a valid capture name"),
-                    None,
-                ))
             }
         }

Once this is done you can do something along the lines of:

(
  (((_ (_) @append_hardline (_)+ @final .)) @whole 
    (#not-match? @whole "\n.") ; prevent matches on multiline expressions
    (#match? @whole ".{50,}") ; where 50 characters is the column limit here
  )
)

It doesn't quite work probably because I'm writing my own cpp query and haven't written it right, but with a single pass I can get:

#include <sstream> // include comment that is very long and will violate line length limit
#include <vector>
#include <sstream>
// include comment that is very long and will violate line length limit
#include <vector>

Probably because my cpp.scm is messed up the second pass ends up not working correctly.

@ZedThree
Copy link
Contributor

ZedThree commented Oct 1, 2024

Am I right in thinking that if I wanted to implement a new language with long-line wrapping, it would (currently) be best to use topiary as a library and implement my own algorithm on top?

@Xophmeister
Copy link
Member Author

@ZedThree Yes, for the time being, you could add long-line wrapping as an external post-processing step 👍

@yannham
Copy link
Member

yannham commented Oct 2, 2024

@aspiwack

If you decide, for instance, that all the forms that cross the maximum column are considered multiline, you'll often make a lot of very short lines. So you need a more complex strategy.

I think it would be quite in line (no pun intended) with the current multiline behavior: if there is just one new line within a complex expression, everything is laid out in multiline mode, which already breaks a 2-line expression into n lines. I think it's also what Rust does, so if you write foo.bar().baz().qux()....verylong() and it wraps, then you do get:

foo
  .bar()
  .baz()
  .qux()
 ...
  ..verylong()

which is IMHO entirely reasonable, and probably the simplest semantics to implement and to understand as a user (that is, is_multiline = has_new_line_in_span() or has_column_higher_than_xx or something).

Do you have a concrete example where this behavior is obviously not what we want?

@aspiwack
Copy link
Member

aspiwack commented Oct 3, 2024

The problem is the something like the following:

foo bar baz (qux | qooks) blam

Where the | indicates the long ligne position. A naive strategy would say that qux qooks is multinline, since it crosses the last allowed column) and will format as

foo
  bar
  baz
  (qux
    qooks)
  blam

While you really want something like:

foo
  bar
  baz
  (qux qooks)
  blam

The art of pretty-printing with line length is to try and minimize the number of vertical breaks that you introduce. Otherwise it simply looks jarring (you can actually go for a minimal solution, it's what Jean-Phillpe Bernardy's prettiest printer is about; but more commonly you would approximate the minimal solution for performance: it just needs to look good enough)

@nbacquey nbacquey removed the epic: contributability Relates to how easy it is for external contributors to contribute to Topiary (engine and queries) label Nov 18, 2024
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

7 participants