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

Consider making foo[[bar]] and foo[bar] never expand when bar is "simple" #199

Open
DavisVaughan opened this issue Jan 22, 2025 · 4 comments

Comments

@DavisVaughan
Copy link
Collaborator

See #183

i.e. with

private$event_callback_counts[[domain]] <- private$event_callback_counts[[domain]] - 1

it should probably never break on the [[ and expand to

private$event_callback_counts[[domain]] <- private$event_callback_counts[[
    domain
  ]] - 
  1

instead it should probably give this, even though the first line exceeds the 80 char limit

private$event_callback_counts[[domain]] <- private$event_callback_counts[[domain]] - 
  1

Related to #163 because we would not want line breaks in $ to force line breaks here!

We will have to nail down what "simple" means, probably:

  • Keywords like TRUE or NULL
  • Identifiers
  • Numerics / integers / complexes
  • etc

Having a list of what makes something "simple" will likely be useful in other places too?

It should probably also respect persistent line breaks if you manually request the expansion? That's quite an edge case though.

@DavisVaughan
Copy link
Collaborator Author

DavisVaughan commented Feb 19, 2025

Now that we've implemented hugging in #228, I think that this may actually be an alternative type of hugging! The rule could be something like:

For [[ and [ calls with exactly 1 argument, the [[ and [ "hug" that argument, never fully expanding.

It's possible this is just a small extension to is_hugging_call()?

That would have helped in dplyr where we have

# before
{
  {
    get_current_group_id = function() {
      duplicate(private[["env_current_group_info"]][["dplyr:::current_group_id"]])
    }
  }
}

# after 
{
  {
    get_current_group_id = function() {
      duplicate(private[["env_current_group_info"]][[
        "dplyr:::current_group_id"
      ]])
    }
  }
}

# ideally
{
  {
    get_current_group_id = function() {
      duplicate(
        private[["env_current_group_info"]][["dplyr:::current_group_id"]]
      )
    }
  }
}

And also

# before
env_current_group_info[["dplyr:::current_group_size"]] <- duplicate(length(private$rows[[group]]))

# after
env_current_group_info[[
  "dplyr:::current_group_size"
]] <- duplicate(length(private$rows[[group]]))

# ideally
env_current_group_info[["dplyr:::current_group_size"]] <- duplicate(length(
  private$rows[[group]]
)) 

Note that as soon as you go past 1 argument, I do feel that not hugging is probably still better (i.e. the current behavior)

# before
env_current_group_info[["dplyr:::current_group_size", a_column]] <- duplicate(length(private$rows[[group]]))

# after
env_current_group_info[[
  "dplyr:::current_group_size",
  a_column
]] <- duplicate(length(private$rows[[group]]))

@lionel-
Copy link
Collaborator

lionel- commented Feb 24, 2025

Now that we've implemented hugging in #228, I think that this may actually be an alternative type of hugging!

Agreed and here is another case that feels very related: #257

@DavisVaughan
Copy link
Collaborator Author

It won't help box much though because we'd probably only apply this to single argument cases, and it seems very common to import multiple things in the [ ], like box::use(a = ./a[f, g]), in which case you'd get expansion again

@lionel-
Copy link
Collaborator

lionel- commented Feb 25, 2025

I only meant to point out another way the hugging notion can be generalised

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