freja icon indicating copy to clipboard operation
freja copied to clipboard

Formatting selected text

Open saikyun opened this issue 4 years ago • 6 comments

(excerpt from chat) if you look at the format function, and instead of sending the whole file (content) you send currently selected text here's the format function: https://github.com/saikyun/freja/blob/4056e765a2922e21e996ffc21676095e981d098c/freja/code_api.janet#L4 in this case, it's commit! followed by extracting text that gets the whole file (that's the same as running commit!) here's a function for getting the selected text: https://github.com/saikyun/freja/blob/main/freja/new_gap_buffer.janet#L988 then you can do something similar to paste in order to remove the selected text and insert the formatted text: https://github.com/saikyun/freja/blob/main/freja/new_gap_buffer.janet#L1012

saikyun avatar Sep 17 '21 09:09 saikyun

Thanks for making this issue.

If it were possible for a reasonable area to be deduced by the editor I think that would be nicer than the user having to select text manually. Although perhaps if there were some kind of "expand selection" functionality may be that could work ok.

sogaiu avatar Sep 17 '21 10:09 sogaiu

What do you mean by "reasonable area"?

Den fre 17 sep. 2021 12:18sogaiu @.***> skrev:

Thanks for making this issue.

If it were possible for a reasonable area to be deduced by the editor I think that would be nicer than the user having to select text manually. Although perhaps if there were some kind of "expand selection" functionality may be that could work ok.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/saikyun/freja/issues/35#issuecomment-921683132, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAS46Z7WX6MBTBZCSXH4OVLUCMIXXANCNFSM5EGUSLMA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

saikyun avatar Sep 17 '21 11:09 saikyun

Ah sorry, that was vague.

Some characteristics I think might go into that could include:

  • Not too large
  • Should not extend beyond what is visible
  • A region that would make sense to format

The first point has to do with not noticing that something has changed because the target area is too big to perceive properly. The larger the selected area, the more likely one might miss some change to it.

The second point has to do with not changing stuff "behind one's back". An issue I have with using automatic formatters is that they can lead to changes to things I am not currently seeing.

The third point has to do with not selecting a region that would succeed at being formatted. For example, (+ 1 doesn't seem like a sensible span of text to try to format.

I don't know if it's possible to achieve these things :)

sogaiu avatar Sep 17 '21 12:09 sogaiu

All right, thanks for clarifying.

Some immediate thoughts:

  • current formatter only works on complete forms, and would only lead to correct results for top level forms
  • thus with the current formatter, the only "reasonable area" easy to implement would be "surrounding top level form"
  • a different formatter will be needed for incremental formatting, and when that is implemented, another kind of partial formatting would be doable

Den fre 17 sep. 2021 14:24sogaiu @.***> skrev:

Ah sorry, that was vague.

Some characteristics I think might go into that could include:

  • Not too large
  • Should not extend beyond what is visible
  • A region that would make sense to format

The first point has to do with not noticing that something has changed because the target area is too big to perceive properly. The larger the selected area, the more likely one might miss some change to it.

The second point has to do with not changing stuff "behind one's back". An issue I have with using automatic formatters is that they can lead to changes to things I am not currently seeing.

The third point has to do with not selecting a region that would succeed at being formatted. For example, (+ 1 doesn't seem like a sensible span of text to try to format.

I don't know if it's possible to achieve these things :)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/saikyun/freja/issues/35#issuecomment-921755703, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAS46ZZG6UYUOTCG5ROZX6TUCMXO7ANCNFSM5EGUSLMA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

saikyun avatar Sep 17 '21 13:09 saikyun

I guess it might be motivation for me to keep my functions short enough to be completely visible in a single window :)

sogaiu avatar Sep 17 '21 13:09 sogaiu

Haha, that's one way to see it. :) I do want to do the partial formatting sometime, but I've understood it's not very easy to do well.

Den fre 17 sep. 2021 15:40sogaiu @.***> skrev:

I guess it might be motivation for me to keep my functions short enough to be completely visible in a single window :)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/saikyun/freja/issues/35#issuecomment-921806213, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAS46Z7NIFDDAWMKRMC36EDUCNAOVANCNFSM5EGUSLMA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

saikyun avatar Sep 17 '21 14:09 saikyun