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

IDLE: Remove FormatParagraph's width setting from config dialog #64776

Closed
taleinat opened this issue Feb 9, 2014 · 20 comments
Closed

IDLE: Remove FormatParagraph's width setting from config dialog #64776

taleinat opened this issue Feb 9, 2014 · 20 comments
Assignees
Labels
topic-IDLE type-feature A feature request or enhancement

Comments

@taleinat
Copy link
Contributor

taleinat commented Feb 9, 2014

BPO 20577
Nosy @terryjreedy, @taleinat
Dependencies
  • bpo-3068: IDLE - Add an extension configuration dialog
  • Files
  • taleinat.20140209.IDLE__Remove_format_paragraph_width_from_config_dialog.patch: initial patch (against branch 3.3; changeset 3e62c6c3977e)
  • formatpara - 20577-34.diff: ready to apply
  • formatpara - 20577-34-v2.diff
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/terryjreedy'
    closed_at = <Date 2014-12-16.08:37:16.115>
    created_at = <Date 2014-02-09.19:28:36.874>
    labels = ['expert-IDLE', 'type-feature']
    title = "IDLE: Remove FormatParagraph's width setting from config dialog"
    updated_at = <Date 2014-12-16.08:37:16.114>
    user = 'https://github.com/taleinat'

    bugs.python.org fields:

    activity = <Date 2014-12-16.08:37:16.114>
    actor = 'terry.reedy'
    assignee = 'terry.reedy'
    closed = True
    closed_date = <Date 2014-12-16.08:37:16.115>
    closer = 'terry.reedy'
    components = ['IDLE']
    creation = <Date 2014-02-09.19:28:36.874>
    creator = 'taleinat'
    dependencies = ['3068']
    files = ['34013', '35562', '35635']
    hgrepos = []
    issue_num = 20577
    keywords = ['patch']
    message_count = 20.0
    messages = ['210779', '220224', '220229', '220230', '220245', '220281', '220286', '220311', '220570', '220574', '220596', '221931', '221941', '222717', '222719', '222732', '222764', '222765', '222786', '232725']
    nosy_count = 3.0
    nosy_names = ['terry.reedy', 'taleinat', 'python-dev']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue20577'
    versions = ['Python 2.7', 'Python 3.4', 'Python 3.5']

    @taleinat
    Copy link
    Contributor Author

    taleinat commented Feb 9, 2014

    Following up bpo-3068, this is a separate patch which moves the "paragraph reformat width" setting from the "general" tab in IDLE's config dialog to the relevant place in the extensions configuration.

    @terryjreedy
    Copy link
    Member

    I updated the patch to match a change, since you posted this, from 70 to 72 in .def and 'maxformatwidth' to ' limit' in formatparagraph. I also added a 'guard' value of 72 in case GetOption returns None (as it did before I changed the option name). I checked that there is no instance of 'para' other than in 'param' in configDialog and ran the tests. I view the attached as ready to commit to 3.4 and merge to 3.5. In 2.7, the patch applies cleanly to the files other that configDialog. The problem there is frameEncoding stuff in the context that is not present in 3.4 or the patch. I would just hand edit the 2.7 file to delete the lines again.

    Would you like to push this as your first commit?

    @terryjreedy terryjreedy added the type-feature A feature request or enhancement label Jun 11, 2014
    @taleinat
    Copy link
    Contributor Author

    Thanks for reviewing this Terry! See two small comments in the review of your patch.

    Indeed, I would very much like for this to be my first commit. Thanks for the pointer WRT backporting to 2.7. Just to make sure, I should commit to 3.5 (default) and then backport to 3.4 and 2.7, correct?

    @terryjreedy
    Copy link
    Member

    Backwards from default was the old, svn way. We hg we merge forward within 3.x. "ready to commit to 3.4 and merge to 3.5.". If you started with 3.5, then you would have to do a null merge of the 3.4 patch into 3.5. It is occasionally done when people decide to backport to maintenacne sometime later, but much nastier.

    Feel free to ask any other questions.

    Review: PEP-8, yes!.

    @taleinat
    Copy link
    Contributor Author

    What do you mean with the comment regarding PEP-8?

    @terryjreedy
    Copy link
    Member

    "For flowing long blocks of text with fewer structural restrictions (docstrings or comments), the line length should be limited to 72 characters."

    @taleinat
    Copy link
    Contributor Author

    I'll be damned. 72 it is, then.

    What about using the 'default' parameter for GetOption() instead of "or 72"?

    @terryjreedy
    Copy link
    Member

    'yes!' meant please do so.

    @taleinat
    Copy link
    Contributor Author

    With very minor changes (just use default=72 instead of "or 72" and added a comment explaining why 72 is the default - see attached), I ran the entire test suite (which is a must according to the dev guide). On current default I'm getting consistent failures in test_tk and test_ttk_guionly, which don't seem related. At least I know I managed to include the tests which require a GUI.

    Can I continue with committing the patch despite these failures? I guess I should report these failures separately?

    @taleinat
    Copy link
    Contributor Author

    Also, this seems like a small change. Should I mention it in Misc/NEWS?

    @terryjreedy
    Copy link
    Member

    General rule: If the other failures happen without the patch, then go ahead. This is, unfortunately, all too common. If they only happen with the patch (extremely unlikely for this one), then try to find out why.

    Yes, this change should have a NEWS item. However, I would not include it with your first patch. On my machine, hg has trouble properly merging changes to NEWS even when there are no real conflicts. I have a couple of other items for committed patches that I intend to push separately very soon and I would include this one if appropriate.

    @terryjreedy
    Copy link
    Member

    Since you are busy, I am planning to commit this tomorrow so I can review the extension config patch with this in place.

    @taleinat
    Copy link
    Contributor Author

    I've been waiting to commit this for some time. I'd really like to do this myself, if you don't mind.

    I'm just waiting for my SSH key to be added, which is taking a long time since apparently all three people who could do so are traveling and unable to help.

    @terryjreedy
    Copy link
    Member

    Ned's comments on bpo-20580, msg222714, reminded me that while each idlelib has its own set of default config files, one set of user files is shared across all installed versions of idle. So we have to be careful about what we do. With this patch, 3.4.2, for instance, could not read the user setting set with 3.4.1. And a setting written by 3.4.2 could not be read by 2.7.8. While I think that is tolerable, let's hold off on this until it is both needed and tested a bit more.

    @terryjreedy
    Copy link
    Member

    This will at least need a What's New entry, though people do not expect new entries in maintenance releases. I think we need an Idle What's New accessible from the help menu and announced on the splash screen. Unlike the standard What's New, it will get occasional entries during a release, and that (and PEP-434) explained at the top.

    @taleinat
    Copy link
    Contributor Author

    I don't think this is a major issue. Most users only use one version of IDLE with one version of Python.

    IDLE's user config is indeed shared between all versions. This is an unfortunate design mistake, and could perhaps be fixed in 3.5.

    Regarding this issue, we could skip 3.4 and include it only in 3.5. I think telling users "fix your IDLE config when switching from 3.4 to 3.5 as so" is reasonable. If we don't allow ourselves this, then without switching to having separate configs for each version of Python, we'll never be able to change anything in IDLE's configuration!

    @terryjreedy
    Copy link
    Member

    The awkwardness of the config design is that idlelib/config-xyz.def files are separate for each version and shared across users (and systems) while .idlerc/config-xyz.def files are separate for each user (account) and shared across versions on the same machine. bpo-20580 is about at least using the existing system-specific sections. Ned also suggested the possibility of version-specific systems. Version specific files are also possible: config-xyz-34.def.

    I am willing to also tell users in 2.8.9 and 3.4.2 to reset any custom formatwidth setting. But we need a way to *tell* users that, even for 3.5. I would like to replace the useless "Type "copyright", "credits" or "license()" for more information." with "See HELP/NEWS for information about changes in each release."

    @taleinat
    Copy link
    Contributor Author

    So is that a go-ahead to commit to the 2.7 and 3.4 branches (as well as default)?

    @terryjreedy
    Copy link
    Member

    What I meant was 'wait until we have an new announcement mechanism', which we very much need anyway, and then apply to all three. And that should have been 'Help/What's New' in the splash announcement. I opened bpo-21961 for this and will try to have a minimal document is a day or so.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Dec 16, 2014

    New changeset 51de0da524b4 by Terry Jan Reedy in branch '2.7':
    Issue bpo-20577: move configuration of FormatParagraph extension to new extension
    https://hg.python.org/cpython/rev/51de0da524b4

    New changeset 3ffa8438d274 by Terry Jan Reedy in branch '3.4':
    Issue bpo-20577: move configuration of FormatParagraph extension to new extension
    https://hg.python.org/cpython/rev/3ffa8438d274

    @terryjreedy terryjreedy self-assigned this Dec 16, 2014
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    topic-IDLE type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants