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.

Sign up for GitHub

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

Jump to bottom

Review ConsolePrompt's method visibilities #1141

Open
mattirn opened this issue Dec 23, 2024 · 2 comments · May be fixed by #1148
Open

Review ConsolePrompt's method visibilities #1141

mattirn opened this issue Dec 23, 2024 · 2 comments · May be fixed by #1148
Labels
enhancement
Milestone
3.28.1

Comments

@mattirn
Copy link
Collaborator

mattirn commented Dec 23, 2024 •
edited
Loading

  1. ConsolePrompt do not implement AutoCloseable
  2. Create open() method that set terminal in raw mode (the last four statement of the constructor)
  3. enclose prompt methods
    1. public Map<String, PromptResultItemIF> prompt(List<AttributedString> header, List<PromptableElementIF> promptableElementList) and
    2. public Map<String, PromptResultItemIF> prompt(List<AttributedString> headerIn, Function<Map<String, PromptResultItemIF>, List<PromptableElementIF>> promptableElementLists)

code with "open() - close()" i.e.

    public Map<String, PromptResultItemIF> prompt(...) throws IOException {
        try {
            open();
            ....
            ....
            return resultMap;
        } finally {
            close();
        }
    }
  1. remove @Deprecated statements
  2. review ConsolePrompt's other methods visibilities
  3. fix examples
@mattirn mattirn mentioned this issue Dec 23, 2024
Merged
@mattirn mattirn added this to the 3.28.1 milestone Dec 23, 2024
@quintesse
Copy link
Contributor

quintesse commented Dec 23, 2024 •
edited
Loading

Ok, so if I understand this correctly this would roll back the change that made ConsolePrompt AutoCloseable?

If so another improvement might be introduce a subclass of ConsolePrompt that does implement AutoCloseable and that handles the open/close for you.

And perhaps (but I haven't thought it through a 100% yet) we would not even need a (public) open() necessarily, thereby maintaining full backward compatibility with older code (but we'd advise people to switch to the newer, improved, AutoCloseable version of ConsolePrompt)

WDYT?

@mattirn
Copy link
Collaborator Author

mattirn commented Jan 6, 2025 •
edited
Loading

Ok, so if I understand this correctly this would roll back the change that made ConsolePrompt AutoCloseable?

Yes.

If so another improvement might be introduce a subclass of ConsolePrompt that does implement AutoCloseable and that handles the open/close for you.

There are only two methods where we need to call open(), close() methods. IMHO there is no real advantage to introduce a AutoCloseable subclass.

And perhaps (but I haven't thought it through a 100% yet) we would not even need a (public) open() necessarily, thereby maintaining full backward compatibility with older code (but we'd advise people to switch to the newer, improved, AutoCloseable version of ConsolePrompt)

I think it is better break now backward compatibility. The old dynamic prompt API is available only in JLine 3.28.0. Those who have started to use old API must migrate to the new one when they start to use JLine version > 3.28.0.

quintesse added a commit to quintesse/jline3 that referenced this issue Jan 8, 2025
@quintesse
Undeprecate ConsolePrompt methods
e8ce2c4 
Fixes jline#1141
@quintesse quintesse linked a pull request Jan 8, 2025 that will close this issue
Open
quintesse added a commit to quintesse/jline3 that referenced this issue Jan 16, 2025
@quintesse
Undeprecate ConsolePrompt methods
cdce944 
Fixes jline#1141
quintesse added a commit to quintesse/jline3 that referenced this issue Jan 16, 2025
@quintesse
Undeprecate ConsolePrompt methods
59cea30 
Fixes jline#1141
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
3.28.1
Development

Successfully merging a pull request may close this issue.

Undeprecate ConsolePrompt methods et al quintesse/jline3
2 participants
@quintesse @mattirn