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

System.Console should provide a GetCursorPosition property to get the position atomically #28535

Closed
damageboy opened this issue Jan 27, 2019 · 13 comments · Fixed by #37559
Closed
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-System.Console
Milestone

Comments

@damageboy
Copy link
Contributor

Currently, the System.Console class in provides two separate properties, CursorLeftand CursorTop that allow the user to change / read the position of the console cursor.

Unfortunately, there seems to be no public API that allows reading the cursor position "atomically" (e.g. both row and column position) although the underlying implementation does get/set the actual position atomically.

Conversly, there does exist a SetCursorPosition public method:
https://github.com/dotnet/corefx/blob/eec001d96a68376c0e504eb7635c8edec196f90f/src/System.Console/src/System/Console.cs#L306

Rational and Usage

The idea would be to be able to read the cursor position on its associated terminal/console in one atomic API call, unlike today, where users of the Console class often write code like:

  var oldX = Console.CursorLeft;
  var oldY = Console.CursotTop;
  Console.SetCursorPosition(0, oldY + 10);
  // Some console output goes out here
  Console.SetCursorPosition(oldX, oldY);

The proposed API would allow reading/setting the position in one API call that would also translate as one atomic system call / write to the underlying OS:

  Console.GetCursorPosition(out var oldX, out var oldY);
  Console.SetCursorPosition(0, oldY + 10);
  // Some console output goes out here
  Console.SetCursorPosition(oldX, oldY);

The proposed change makes reading the console cursor position more readable, efficient (more on that later) and most importantly atomic, in the sense that the API will allow to ensure that the console cursor is/was in the reported position, unlike today, where the console cursor position can change in between calls to the getters/setters of CursorLeft and CursorTop.

Proposed API

namespace System
{
    public static class Console 
    {
        void GetCursorPosition(out int left, out int top);
    }
}

Details

@damageboy damageboy changed the title Provide Canonical API to read console cursor position in one call Provide canonical API to get/set console cursor position in one call Jan 27, 2019
@danmoseley
Copy link
Member

Could you please format like an API proposal (info in docs folder)? That will make API review meeting quicker.

@danmoseley
Copy link
Member

@wtgodbe owns this area.

If on at least one platform we can get the value atomically, this seems like a reasonable idea to me.

@danmoseley
Copy link
Member

I am not sure the best type as we seem to avoid adding API with tuples (based on a discussion I can't find right now). Point would be nice but it's in winforms.

@stephentoub
Copy link
Member

Point would be nice but it's in winforms.

It's in System.Drawing.Primitives in corefx.

@damageboy
Copy link
Contributor Author

@danmosemsft Will do so (docs).

Currently both ConsolePal.{Unix,Windows}.cs provide an atomic one. The Unix / VT100 one is the super tricky one to nail...

@damageboy damageboy changed the title Provide canonical API to get/set console cursor position in one call System.Console should provide a CursorPosition property to get/set the position atomically Jan 27, 2019
@danmoseley
Copy link
Member

@stephentoub thank you, maybe I should not comment on phone where I can't look.

@stephentoub
Copy link
Member

😄

@damageboy damageboy changed the title System.Console should provide a CursorPosition property to get/set the position atomically System.Console should provide a GetCursorPosition property to get the position atomically Jan 28, 2019
@GSPP
Copy link

GSPP commented Jan 28, 2019

This API should return a struct to not force the caller to accept side-effects with those out parameters. Expressions compose better than statements.

@damageboy
Copy link
Contributor Author

@GSPP I know what you are talking about, but I thought that it would be slightly weird to have an impedance mismatch between the Set and the Get methods... and since the old API isn't going away...

Do you feel I should re-write this into TWO NEW methods accepting a single struct?

@tannergooding
Copy link
Member

This API should return a struct to not force the caller to accept side-effects with those out parameters

What side-effects are you referring to? The overall behavior (and work) for whether you return a struct or two ints is effectively the same.

Returning a struct requires exposing additional public surface area, however.

@damageboy
Copy link
Contributor Author

@tannergooding I can't find the exact link and therefore the exact name for "this", but I think @GSPP is referring to the fact that out parameters force the JIT to output memory fences/barriers in case the output is a member of a class on the heap.

I also might be completely wrong in my interpretation... :)

@GSPP
Copy link

GSPP commented Jan 29, 2019

out forces the caller to create variables and to assign to them. That's the side-effect I meant. I am not talking about global state mutation.

Before C# got out var this was much more clunky than it is now. But it still feels impure to me from the perspective of preferring functional code over imperative code. Using functional constructs is a very powerful heuristic for increasing code quality. Expressions are often preferable to statements. Dataflow preferable to side-effects. Pure functions over impure functions. Etc.

For example, out prevents us from treating the result of the function call as any other normal value. We can't pass it around anymore. Essentially, out forces deconstruction.

I also would argue that creating a small struct ConsoleCoordinates is small surface area, very stable over time and easily understood by users.

Maybe this illustrates the point: It is suboptimal API design that int.TryParse returns bool and has an out parameter. This always lead to very clunky code before we got out var. It prevented usage in expression contexts such as LINQ. Better is to return int? (which was not available at the time).

out clearly has it's places but here it seems unnecessary.

Hope these thoughts make sense.

@msftgits msftgits transferred this issue from dotnet/corefx Feb 1, 2020
@msftgits msftgits added this to the 5.0 milestone Feb 1, 2020
@maryamariyan maryamariyan added the untriaged New issue has not been triaged by the area owner label Feb 23, 2020
@carlossanlop carlossanlop added api-ready-for-review and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation untriaged New issue has not been triaged by the area owner labels Apr 27, 2020
@carlossanlop carlossanlop modified the milestones: 5.0, Future Apr 27, 2020
@bartonjs
Copy link
Member

bartonjs commented Jun 5, 2020

Approved with modifications

namespace System
{
    public static class Console 
    {
        public static (int Left, int Top) GetCursorPosition();
    }
}

@bartonjs bartonjs added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review labels Jun 5, 2020
@stephentoub stephentoub self-assigned this Jun 7, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Console
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants