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

Translate cursor position #93

Closed
glenjamin opened this issue Jan 11, 2017 · 8 comments
Closed

Translate cursor position #93

glenjamin opened this issue Jan 11, 2017 · 8 comments
Labels
locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. status:needs discussion Issues needing discussion and a decision to be made before action can be taken

Comments

@glenjamin
Copy link

I'm not quite sure what the API / CLI would look like for this, but it'd be cool
If you could pass in the current cursor position and be told where the equivalent position is in the output.

This would be for editor plugins so you can prettify without losing your spot.

@wmertens
Copy link

OTOH, not implementing this simply means that in some cases your cursor is on a very nearby position while doing incremental prettifying

@vramana vramana added the status:needs discussion Issues needing discussion and a decision to be made before action can be taken label Jan 11, 2017
@bnjmnt4n
Copy link
Contributor

I don't think this would be easy to do with Prettier, as it does not generate a sourcemap. With a sourcemap, the cursor position can be located at a specific AST node, and the sourcemap can be used to find the resulting node. However, without that, the only easy way would be to reparse the generated code to regenerate an identical AST with the new location properties, and map from the old node to the new node, but that would be bad performance-wise.

@jlongster
Copy link
Member

Yeah, we would need something like sourcmaps to make this work. We don't support them now, but we might in the future to also allow prettier to be a frontend to other build tools that want to generate code from ASTs. It's probably going to be a lot of work though.

I'm worried that sourcemaps is going to decrease performance a lot, which makes it harder for the in-editor formatting use case. I'm not sure it's worth the perf hit to maintain cursor position, but I suppose it could be optional. Generating sourcemaps is typically very heavy so I'd expect to see a 20-30% perf decrease.

@ftlio
Copy link

ftlio commented Feb 14, 2017

Does anyone have a temporary solution for emacs? It doesn't have to be exact, but in a class definition the cursor will jump to the top of the class. The before-save-hook has some pretty frustrating quality of life repercussions.

@vjeux
Copy link
Contributor

vjeux commented Mar 13, 2017

We've been thinking about this with @cpojer and we found a solution that should work.

  1. After we have the AST, use the same binary-search technique as comments in order to find what AST node the cursor position is in.
  2. In the generic print function, if we are about to print that node, then add a special cursor token with the offset the cursor was compared to the ast node.
  3. When we are about to print that token, the new cursor should be at the token position + delta.

If anyone is interesting in trying it out, that would be great! cc @rattrayalex @jlongster

@EnoahNetzach
Copy link

If no one is around, I could take a try at this :3

josephfrazier added a commit to josephfrazier/prettier that referenced this issue May 20, 2017
This roughly implements the algorithm from prettier#93 (comment)
@josephfrazier
Copy link
Collaborator

I decided to give this a shot at #1637. If you're interested in having this feature, I'd appreciate feedback :)

josephfrazier added a commit to josephfrazier/prettier that referenced this issue May 30, 2017
This addresses prettier#93 by
adding a new option, `cursorOffset`, that tells prettier to determine
the location of the cursor after the code has been formatted. This is
accessible through the API via a new function, `formatWithCursor`, which
returns a `{formatted: string, cursorOffset: ?number}`.

Here's a usage example:

```js
require("prettier").formatWithCursor(" 1", { cursorOffset: 2 });
// -> { formatted: '1;\n', cursorOffset: 1 }
```
vjeux pushed a commit that referenced this issue Jun 1, 2017
* Add `formatWithCursor` API with `cursorOffset` option

This addresses #93 by
adding a new option, `cursorOffset`, that tells prettier to determine
the location of the cursor after the code has been formatted. This is
accessible through the API via a new function, `formatWithCursor`, which
returns a `{formatted: string, cursorOffset: ?number}`.

Here's a usage example:

```js
require("prettier").formatWithCursor(" 1", { cursorOffset: 2 });
// -> { formatted: '1;\n', cursorOffset: 1 }
```

* Add `--cursor-offset` CLI option

It will print out the offset instead of the formatted output. This
makes it easier to test. For example:

    echo ' 1' | prettier --stdin --cursor-offset 2
    # prints 1

* Add basic test of cursor translation

* Document `cursorOffset` option and `formatWithCursor()`

* Print translated cursor offset to stderr when --cursor-offset is given

This lets us continue to print the formatted code, while also
communicating the updated cursor position.

See #1637 (comment)

* doc-print cursor placeholder in comments.printComments()

See #1637 (comment)

* Compare array index to -1 instead of >= 0 to determine element presence

See #1637 (comment)

* Return {formatted, cursor} from printDocToString() instead of mutating options

See #1637 (comment)
@vjeux vjeux closed this as completed Jul 6, 2017
@vjeux
Copy link
Contributor

vjeux commented Jul 6, 2017

It's been built :)

@lock lock bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label Jul 7, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Jul 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. status:needs discussion Issues needing discussion and a decision to be made before action can be taken
Projects
None yet
Development

No branches or pull requests

9 participants