Skip to content
This repository has been archived by the owner on Jan 15, 2020. It is now read-only.

Diff parser error #79

Open
2 of 3 tasks
Kmaschta opened this issue Mar 17, 2017 · 8 comments · Fixed by #83
Open
2 of 3 tasks

Diff parser error #79

Kmaschta opened this issue Mar 17, 2017 · 8 comments · Fixed by #83
Labels
Milestone

Comments

@Kmaschta
Copy link
Contributor

Kmaschta commented Mar 17, 2017

Description

@jpetitcolas really is the nemesis of Sedy.

Additional informations

Inefficient diff parser error
{
    hunk: '@@ -1,3 +1,13 @@\n+---\n+layout: post\n+title: "Les générateurs"\n+excerpt: "Les générateurs sont une fonctionnalité introduite dans es6 qui permet de créer des fonctions spéciales avec la capacité de mettre en pause leur éxécution en retournant un résultat intermédiaire."\n+author: <a href="https://github.com/ThieryMichel">Thiery Michel</a>\n+tags:\n+- es6\n+- generator\n+---\n+\n # Les générateurs\n \n ## Que sont les générateurs',
    position: null
}

TODO

  • Do not throw an error or try/catch the error to let other commits to be pushed
  • Log the related comment too
  • See why GitHub do not sent a position this time
@Kmaschta
Copy link
Contributor Author

Kmaschta commented Apr 13, 2017

Another inefficient diff parser error
{
    hunk: '@@ -44,15 +44,21 @@ Lastly, filesystems don\'t order files the same way. Some filesystems don\'t even\n \n You probably have a different configuration on the `development` environment, and on the `test` environment. This can be `ENV` vars, API endpoints, databases, browsers, etc.\n \n-One of the first things to do to reproduce a CI build locally is to use exaclty the same setup as the CI. If your build setup is good, it should be as easy as running:\n+to reproduce a CI build locally, one of the first things to do is to use exaclty the same setup as the CI. If your build automation is good, it should be as easy as running:',
    position: null
}

@Kmaschta
Copy link
Contributor Author

Hint for the position = null, it can be a review submitted from a commit no from the PR "files changed".

@Kmaschta
Copy link
Contributor Author

@Kmaschta Kmaschta added the bug label May 24, 2017
Kmaschta added a commit that referenced this issue May 24, 2017
When we try a sed comment from a commit (and not from the `Files changed` tab),
and/or we write a sed comment from the deletion side of the review, the
GitHub API might not send the `position` of the diff hunk but its
`original_position`.

This PR should fix #79
@Kmaschta
Copy link
Contributor Author

I re-open this issue since I have some new elements.
I also reverted what I thought it was the solution. #86

@Kmaschta Kmaschta reopened this May 31, 2017
@Kmaschta
Copy link
Contributor Author

Another one marmelab/comfygure#90 (review) :

{
    hunk: '@@ -24,31 +34,39 @@ const ls = (ui, modules) => function* () {\n };\n \n const add = (ui, modules, options) => function* () {\n- const { red, bold } = ui.colors;\n+ const { red, bold, green } = ui.colors;\n \n if (!options.length) {\n ui.error(`${red(\'No environment specified.\')}`);\n- help(ui, 1);\n }\n \n if (options.length > 1) {\n ui.error(`${red(\'Invalid environment format. The environment name should be one word.\')}`);\n- help(ui, 1);\n+ }\n+\n+ if (options.length !== 1) {\n+ ui.print(`${bold(\'SYNOPSIS\')}\n+ ${bold(\'comfy\')} env add <environment>\n+\n+Type ${green(\'comfy env --help\')} for details`);\n+\n+ return ui.exit(0);\n }\n \n const project = yield modules.project.retrieveFromConfig();\n const environment = yield modules.environment.add(project, options[0]);\n- const addCommand = `comfy add ${environment.name}`;\n+ const addCommand = `comfy setall ${environment.name}`;\n \n- ui.print(`${bold(\'Cool!\')} Your new environment "${bold(environment.name)}" was successfully saved.`);\n- ui.print(`You can now add a configuration, try ${bold(addCommand)}`);\n+ ui.print(`${bold(green(\'Environment successfully created\'))}`);\n+ ui.print(`You can now set a configuration fot this environment using ${bold(addCommand)}`);',
    position: 71
}

@Kmaschta
Copy link
Contributor Author

Here is another bug: https://github.com/marmelab/admin-on-rest/pull/1521#discussion_r168098179

{
    hunk: '@@ -109,6 +110,106 @@ export const PostList = (props) => (\n );\n ```\n \n+### Bulk Actions\n+\n+You can replace the list of default bulk actions by your own element using the `bulkActions` prop:',
    position: 14
}

@Luwangel

This comment has been minimized.

@Kmaschta
Copy link
Contributor Author

For this issue, I get sometimes weird positions from the GitHub API.

I opened a thread on the GitHub Platform forum.

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

Successfully merging a pull request may close this issue.

2 participants