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

Add preferences toggle (or new shortcut) for the forthcoming (and now in R-devel) native pipe symbol (|>) #8534

Closed
hrbrmstr opened this issue Dec 4, 2020 · 26 comments
Assignees

Comments

@hrbrmstr
Copy link
Contributor

@hrbrmstr hrbrmstr commented Dec 4, 2020

While not a trivial feature request ask, a toggle to have cmd-shift-M use the new native pipe symbol vs {magrittr}'s would be great! The suggestion for an alternate keybinding by @Robinlovelace (in the parallel feature req) for a separate keybinding would be a fine alternative as well, cmd-shift-> (greater than) seems to be available at least on macOS and seems like a pretty good one.

A cosmetic A++ parallel request to this is to figure out why |> does not render ligatured (as a right-facing triangle) as it does in Sublime, iTerm, VS Code, etc. (this may be an un-fixable-by-RStudio QT issue, tho).

The ligature rendering works fine when backtick-quoted or single/double quoted.

image

@Robinlovelace
Copy link

@Robinlovelace Robinlovelace commented Dec 4, 2020

Suggested ways it could be supported (from user perspective, without comments on implementation which is beyond my ken):

@Robinlovelace
Copy link

@Robinlovelace Robinlovelace commented Dec 4, 2020

Just a comment on accessibility from Barry Rowlingson:

Obscuring underlying character codes with ligatures is not good for accessibility. Don't do it.

Implication: any changes should not become the default and people should think twice before setting ligatures.

@hrbrmstr
Copy link
Contributor Author

@hrbrmstr hrbrmstr commented Dec 4, 2020

Um, ligatures are a personal choice. Nobody has to use a ligature-enabled font. Barry's comment is woefully uninformed.

@Robinlovelace
Copy link

@Robinlovelace Robinlovelace commented Dec 4, 2020

Um, ligatures are a personal choice. Nobody has to use a ligature-enabled font. Barry's comment is woefully uninformed.

I don't know much about it. Is there any evidence about it? The reason for raising this was that it seemed to ring try: if newbies start seeing triangles and arrows could that not make R seem harder than it is? Slightly off topic but interesting ideas both ways.

@hrbrmstr
Copy link
Contributor Author

@hrbrmstr hrbrmstr commented Dec 4, 2020

@Robinlovelace
Copy link

@Robinlovelace Robinlovelace commented Dec 4, 2020

I see your point. Barry's comment on Twitter was a bit dictatorial. Hopefully my suggestion that

any changes should not become the default and people should think twice before setting ligatures

is more nuanced and compatible with that - agree, I'm 'pro choice' on this ; )

@jmcphers
Copy link
Member

@jmcphers jmcphers commented Dec 4, 2020

Ligature fonts are so popular that we briefly entertained the idea of shipping a couple with RStudio Server, but decided against it for the reasons @Robinlovelace mentioned. However we'd like them to work correctly for folks who prefer them.

@hrbrmstr Feel free to file a separate issue on the ligature. We'll need to know what specific ligature font you're using, too.

@kevinushey
Copy link
Contributor

@kevinushey kevinushey commented Dec 4, 2020

The issue with ligatures will be fixed by #8533 (we need to ensure |> is tokenized as a single token, which is done in that PR).

Having Cmd + Shift + M use the new native pipe operator wouldn't be too difficult:

@SuppressWarnings("deprecation")
public void insertPipeOperator()
{
boolean hasWhitespaceBefore =
Character.isSpace(getCharacterBeforeCursor()) ||
(!hasSelection() && getCursorPosition().getColumn() == 0);
if (hasWhitespaceBefore)
insertCode("%>% ", false);
else
insertCode(" %>% ", false);
}

We'd just have to use the native pipe operator if the user is running R (>= 4.1.0), or if they've explicitly opted in to using it.

@Robinlovelace
Copy link

@Robinlovelace Robinlovelace commented Dec 4, 2020

Wow fast work guys, seems the solutions are in place less than 24 hours after R's 'native pipe' was available!

@ntluong95
Copy link

@ntluong95 ntluong95 commented Dec 4, 2020

sorry @kevinushey. I updated the newest version of Rstudio and R-devel but I can not display the native pipe like that. Also we should consider about the short cut for it. It should be different with Ctrl + Shift + M

@Robinlovelace
Copy link

@Robinlovelace Robinlovelace commented Dec 4, 2020

Ctrl + Shift + P gets my vote

@ntluong95
Copy link

@ntluong95 ntluong95 commented Dec 4, 2020

That makes sense

@gtritchie
Copy link
Member

@gtritchie gtritchie commented Dec 4, 2020

Ctrl+Shift+P is already used by the new command-palette feature in 1.4.

@Robinlovelace
Copy link

@Robinlovelace Robinlovelace commented Dec 4, 2020

Aha. M is my vote then, unless there are reasons not to go for that 👍

@andrie
Copy link
Member

@andrie andrie commented Dec 7, 2020

I suspect that there will be quite a long transition period where the magrittr pipe %>% as well as the base R pipe |> will be in use.

Thus I suggest the keyboard shortcut Shift + Ctrl + | as the keyboard shortcut for the base R pipe.

@ntluong95
Copy link

@ntluong95 ntluong95 commented Dec 7, 2020

I think Ctrl + Shift + > may be easier to remember as ">" symbol meaning forward. In addition, the symbol ">" is near with letter M

@andrie
Copy link
Member

@andrie andrie commented Dec 7, 2020

The shortcut Ctrl + Shift + > is already in use to surface a quick change of open tabs:

image

@hadley
Copy link
Member

@hadley hadley commented Dec 7, 2020

I would recommend making the existing pipe shortcut configurable rather than adding a new keyboard shortcut. It should definitely require user opt-in, since while the magrittr 2.0.0 pipe and the base pipe are very similar, there are a few differences on edge cases that might catch folks out.

@mikebessuille mikebessuille added this to the v1.4-juliet-rose milestone Dec 7, 2020
@mikebessuille
Copy link
Contributor

@mikebessuille mikebessuille commented Dec 7, 2020

To consider for 1.4-Juliet-Rose or 1.5.

@kevinushey
Copy link
Contributor

@kevinushey kevinushey commented Dec 7, 2020

@hadley: do you imagine that in the future, use of the native pipe over the magrittr pipe will be encouraged in the tidyverse? Or will they just be two potential alternatives based on what the user wants to do?

@kevinushey
Copy link
Contributor

@kevinushey kevinushey commented Dec 7, 2020

(Another way of putting it: will some future version of tidyverse documentation use |> over %>%?)

@hadley
Copy link
Member

@hadley hadley commented Dec 7, 2020

@kevinushey the plan is to gradually switch to use the base pipe. Unfortunately we don't have an exact plan for how to do while keeping our guarantee that the tidyverse works with the last five versions of R. We'll probably need to do some \Sexpr{} magic in examples so that new versions of R see |> and old versions see %>%.

@jmcphers
Copy link
Member

@jmcphers jmcphers commented Dec 18, 2020

This is now implemented in the Juliet Rose branch.

@astayleraz
Copy link
Contributor

@astayleraz astayleraz commented Jan 25, 2021

Verified setting shows now in Version 1.4.1507-1 and R 3.6.0. Added QA automation test to verify changing this setting and the output text that shows when the hot key command is used.

@gforge
Copy link

@gforge gforge commented May 23, 2021

Column name hints are not yet available when using |> (version 1.4.1714)

@mikebessuille
Copy link
Contributor

@mikebessuille mikebessuille commented May 25, 2021

@gforge that is covered separately in this issue: #9385

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests